diff --git a/api/v4/source/access_control.yaml b/api/v4/source/access_control.yaml index 6b960ff94d74..57475a8ec516 100644 --- a/api/v4/source/access_control.yaml +++ b/api/v4/source/access_control.yaml @@ -675,3 +675,44 @@ $ref: "#/components/responses/InternalServerError" "501": $ref: "#/components/responses/NotImplemented" + /api/v4/access_control/decisions/actions/search: + post: + tags: + - access control + summary: Search allowed actions for the current user (render-time decision) + description: | + Returns non-authoritative, render-time ABAC decisions for the current + session user on a given resource and set of actions. Use these decisions + to decide whether to show or hide UI controls before the user attempts + an action. + + The subject is always the authenticated session user; there is no way to + probe another user's decisions. Server-side enforcement always re-evaluates + the full policy on the actual request and remains the source of truth. + + Gated by the `PermissionPolicies` feature flag and an Enterprise license. + When ABAC is inactive for the resource, every action is returned as + `allowed: true, evaluated: true`. + + ##### Permissions + Must be authenticated (any logged-in user). + operationId: SearchAccessControlDecisionActions + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/ActionSearchRequest" + responses: + "200": + description: Render-time action decisions returned successfully. + content: + application/json: + schema: + $ref: "#/components/schemas/ActionSearchResponse" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "500": + $ref: "#/components/responses/InternalServerError" diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index a4ff44c48304..6f2c86a699ec 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -5446,6 +5446,120 @@ components: type: integer format: int64 description: The time in milliseconds the recap channel was created + AccessControlResource: + type: object + description: Identifies a resource (type + id) for an ABAC decision request. + required: + - type + - id + properties: + type: + type: string + description: The resource type (e.g. "channel"). + id: + type: string + description: The resource identifier. + RenderPermissionDecision: + type: object + description: | + A non-authoritative, render-time ABAC decision for a single action. + Must not be used to authorize an action — server enforcement is authoritative. + required: + - allowed + - evaluated + properties: + allowed: + type: boolean + description: Whether the action is permitted for rendering purposes. + evaluated: + type: boolean + description: Whether the server intentionally computed this decision. + reason: + type: string + description: Generic denial reason (e.g. "restricted_by_policy"). Never contains policy names, expressions, or attribute values. + ActionSearchResult: + type: object + required: + - action + properties: + action: + type: object + required: + - name + properties: + name: + type: string + description: Name of a permitted action. Denial is expressed by omission from the results list. + ActionSearchSubject: + type: object + description: > + RESERVED for Phase 3 cross-subject evaluation. In Phase 2 the ID must equal + the authenticated session user ID; mismatches return 403. Present to allow + Phase 3 as a non-breaking extension. + required: + - id + properties: + id: + type: string + type: + type: string + ActionSearchPage: + type: object + description: > + RESERVED for Phase 3 pagination. Accepted in requests but always ignored; + next_token is never emitted in responses. Present to allow Phase 3 + pagination as a non-breaking extension. + properties: + next_token: + type: string + ActionSearchRequest: + type: object + description: Request body for the Action Search render-decision endpoint. + required: + - resource + properties: + resource: + $ref: "#/components/schemas/AccessControlResource" + actions: + type: array + description: > + Actions to evaluate (max 16). Omit or leave empty for discovery mode: + the server evaluates all registered actions for the resource type and + returns the permitted set. + maxItems: 16 + items: + type: string + subject: + $ref: "#/components/schemas/ActionSearchSubject" + page: + $ref: "#/components/schemas/ActionSearchPage" + ActionSearchResponse: + type: object + description: Response from the Action Search render-decision endpoint. + required: + - resource + - results + - decisions + properties: + resource: + $ref: "#/components/schemas/AccessControlResource" + results: + type: array + description: > + AuthZEN-canonical list: only PERMITTED actions appear here. + Denial is expressed by omission. Always present; an empty array [] + is meaningful (all evaluated actions were denied). + items: + $ref: "#/components/schemas/ActionSearchResult" + decisions: + type: object + description: > + Mattermost extension: all evaluated actions with full decision detail + (allowed, evaluated, reason), including denied ones. Always present. + additionalProperties: + $ref: "#/components/schemas/RenderPermissionDecision" + page: + $ref: "#/components/schemas/ActionSearchPage" RecapLimitStatus: type: object description: The current user's recap limit status including usage and cooldown information diff --git a/e2e-tests/playwright/lib/src/index.ts b/e2e-tests/playwright/lib/src/index.ts index ef6df2b774a6..67c7e09582ba 100644 --- a/e2e-tests/playwright/lib/src/index.ts +++ b/e2e-tests/playwright/lib/src/index.ts @@ -109,6 +109,8 @@ export {setWysiwygUserPreference, WYSIWYG_PREF_CATEGORY, WYSIWYG_PREF_NAME} from export {TextInputSetting} from './ui/components/system_console/base_components'; +export {expectFilesVisible, expectFilesRedacted} from './ui/components/channels/post'; + export {TestArgs, ScreenshotOptions} from './types'; export { diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/center_view.ts b/e2e-tests/playwright/lib/src/ui/components/channels/center_view.ts index 46d77ac96ee1..75abcc9a8821 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/center_view.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/center_view.ts @@ -79,6 +79,16 @@ export default class ChannelsCenterView { return new ChannelsPost(lastPost); } + /** + * Return the Center post whose body contains the given text. Prefer this over getLastPost: + * adding a member appends a join system message, so the post under test is often not last. + */ + async getPostByText(text: string) { + const post = this.container.getByTestId('postView').filter({hasText: text}).last(); + await post.waitFor(); + return new ChannelsPost(post); + } + /** * Return the ID of the last post in the Center */ diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/post.ts b/e2e-tests/playwright/lib/src/ui/components/channels/post.ts index d98b60bd9a95..3ca6c99f8635 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/post.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/post.ts @@ -10,6 +10,18 @@ import BurnOnReadTimerChip from './burn_on_read_timer_chip'; import PostMenu from './post_menu'; import ThreadFooter from './thread_footer'; +// Both assert the positive case first: a lone "placeholder is absent" check also passes +// against a region that has not rendered at all. +export async function expectFilesVisible(scope: Locator) { + await expect(scope.getByTestId('fileAttachmentList')).toBeVisible(); + await expect(scope.getByTestId('redactedFilesPlaceholder')).toHaveCount(0); +} + +export async function expectFilesRedacted(scope: Locator) { + await expect(scope.getByTestId('redactedFilesPlaceholder')).toBeVisible(); + await expect(scope.getByTestId('fileAttachmentList')).toHaveCount(0); +} + export default class ChannelsPost { readonly container: Locator; @@ -29,6 +41,12 @@ export default class ChannelsPost { readonly burnOnReadTimerChip; readonly concealedPlaceholder; + // File attachments and their ABAC-redacted stand-in + readonly fileAttachmentList; + readonly redactedFilesPlaceholder; + + readonly postPreview; + constructor(container: Locator) { this.container = container; @@ -50,6 +68,13 @@ export default class ChannelsPost { this.concealedPlaceholder = new BurnOnReadConcealedPlaceholder( container.getByTestId(/^burn-on-read-concealed-/), ); + + this.fileAttachmentList = container.getByTestId('fileAttachmentList'); + this.redactedFilesPlaceholder = container.getByTestId('redactedFilesPlaceholder'); + + // The embedded permalink preview carries no test id, so the class name is the + // only handle available. + this.postPreview = container.locator('.post-preview'); } async toBeVisible() { @@ -143,6 +168,20 @@ export default class ChannelsPost { await expect(this.container).not.toContainText(text); } + /** + * @param scope Sub-region to assert within, e.g. an embedded permalink preview + */ + async toHaveFilesVisible(scope: Locator = this.container) { + await expectFilesVisible(scope); + } + + /** + * @param scope Sub-region to assert within, e.g. an embedded permalink preview + */ + async toHaveFilesRedacted(scope: Locator = this.container) { + await expectFilesRedacted(scope); + } + /** * Check if this is a burn-on-read post */ diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_download.spec.ts b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_download.spec.ts deleted file mode 100644 index 61f0bb68f1fb..000000000000 --- a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_download.spec.ts +++ /dev/null @@ -1,416 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import { - expect, - test, - enableABAC, - getAdminClient, - TestBrowser, - getRandomId, - testConfig, -} from '@mattermost/playwright-lib'; - -import type {CustomProfileAttribute} from '../../../channels/custom_profile_attributes/helpers'; -import {setupCustomProfileAttributeFields} from '../../../channels/custom_profile_attributes/helpers'; -import { - createUserForABAC, - createPrivateChannelForABAC, - createPermissionPolicy, - deletePermissionPolicyByName, - enableUserManagedAttributes, - navigateToPermissionPoliciesPage, -} from '../support'; - -import {setupUserAndChannel} from './helpers'; - -/** - * ABAC Permission Policies - Download File Runtime Enforcement (MM-64508) - * - * Tests that permission policies for download_file_attachment are correctly - * enforced in the channel UI. Covers the straightforward deny/allow pair, the - * attribute-matching flow, and the Burn-on-Read + permalink edge cases. - * - * CEL strategy: - * - DENIED tests: celExpression = 'false' → unconditional deny, no attribute dependency - * - ALLOWED tests: no permission policy created → no-policy = implicit allow - * - * Cleanup strategy per describe block: - * - `let lastPolicyName` tracks the name of whatever policy the current test created - * - `afterEach` deletes it by exact name via deletePermissionPolicyByName (reliable) - * - `beforeEach` also deletes by name in case afterEach failed (safety net) - * - Individual tests just set lastPolicyName and do NOT do their own cleanup - */ - -// ─── Download Enforcement ──────────────────────────────────────────────────── - -test.describe('ABAC Permission Policies - Download File Enforcement', () => { - let lastPolicyName = ''; - let savedAdminClient: any = null; - - test.afterEach(async () => { - if (lastPolicyName && savedAdminClient) { - await deletePermissionPolicyByName(savedAdminClient, lastPolicyName); - lastPolicyName = ''; - savedAdminClient = null; - } - }); - - test('MM-T5820 user denied download sees redacted placeholder instead of file', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); - - const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); - await adminChannelsPage.goto(team.name, channelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.postMessage('File attachment post', ['sample_text_file.txt']); - - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await navigateToPermissionPoliciesPage(systemConsolePage.page); - - lastPolicyName = `Download Deny ${pw.random.id()}`; - await createPermissionPolicy(systemConsolePage.page, { - name: lastPolicyName, - celExpression: 'false', - permissions: ['Download Files'], - adminClient, - }); - - // Re-apply ABAC guard: a concurrent initSetup() may have reset - // AccessControlSettings.EnableAttributeBasedAccessControl to false between - // enableABAC() above and the denied user's login, preventing enforcement. - await adminClient.patchConfig({ - AccessControlSettings: {EnableAttributeBasedAccessControl: true}, - } as any); - await expect - .poll( - async () => { - const cfg = await adminClient.getConfig(); - return cfg.AccessControlSettings?.EnableAttributeBasedAccessControl === true; - }, - {timeout: 15000, intervals: [500, 1000, 2000]}, - ) - .toBe(true); - - const {channelsPage: deniedChannelsPage, page: deniedPage} = await pw.testBrowser.login(deniedUser); - await deniedChannelsPage.goto(team.name, channelName); - await deniedChannelsPage.toBeVisible(); - - await expect(deniedPage.getByTestId('redactedFilesPlaceholder')).toBeVisible({timeout: 15000}); - await expect(deniedPage.getByTestId('redactedFilesPlaceholder')).toContainText('Files not available'); - await expect(deniedPage.locator('[data-testid="fileAttachmentList"]')).not.toBeVisible(); - }); - - test('MM-T5821 user sees file normally when no download restriction policy exists', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - // lastPolicyName is '' here — no policy to create, beforeEach cleaned any stale one - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser, channelName} = await setupUserAndChannel(adminClient, team); - - const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); - await adminChannelsPage.goto(team.name, channelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.postMessage('File attachment post', ['sample_text_file.txt']); - - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await systemConsolePage.page.waitForTimeout(1000); - - const {channelsPage: userChannelsPage, page: userPage} = await pw.testBrowser.login(testUser); - await userChannelsPage.goto(team.name, channelName); - await userChannelsPage.toBeVisible(); - - await expect(userPage.locator('[data-testid="fileAttachmentList"]')).toBeVisible({timeout: 15000}); - await expect(userPage.getByTestId('redactedFilesPlaceholder')).not.toBeVisible(); - }); -}); - -// ─── Attribute-Based Policy — Matching User ─────────────────────────────────── - -/** - * MM-T5826 split into two tests (_a denied, _b allowed) that share a - * beforeAll. The beforeAll pays the 31-second AttributeView gate plus the - * policy-creation UI work ONCE. Each test then just logs the relevant user - * in and asserts the file visibility. - */ -test.describe('ABAC Permission Policies - Attribute-Based Access - MM-T5826', () => { - let sharedAdminClient: any = null; - let sharedPolicyName = ''; - let sharedTeam: any; - let sharedChannelName = ''; - let userAllowed: Awaited>; - let userDenied: Awaited>; - let licensed = true; - let sharedTestBrowser: TestBrowser | null = null; - - test.beforeAll(async ({browser}) => { - test.setTimeout(240000); - - const {adminClient, adminUser} = await getAdminClient(); - if (!adminUser) { - throw new Error('Admin user not found — cannot proceed with ABAC file-access tests'); - } - sharedAdminClient = adminClient; - - try { - const lic = await adminClient.getClientLicenseOld(); - if (!lic || lic.IsLicensed !== 'true') { - licensed = false; - return; - } - } catch { - licensed = false; - return; - } - - // Wait 31s to guarantee the server-side AttributeView 30-second refresh - // gate has expired before creating users with attributes. - await new Promise((resolve) => setTimeout(resolve, 31000)); - - await enableUserManagedAttributes(adminClient); - const departmentAttr: CustomProfileAttribute[] = [{name: 'Department', type: 'text', value: ''}]; - const attributeFieldsMap = await setupCustomProfileAttributeFields(adminClient, departmentAttr); - - userAllowed = await createUserForABAC(adminClient, attributeFieldsMap, [ - {name: 'Department', type: 'text', value: 'Engineering'}, - ]); - userDenied = await createUserForABAC(adminClient, attributeFieldsMap, [ - {name: 'Department', type: 'text', value: 'Sales'}, - ]); - - const suffix = getRandomId(); - sharedTeam = await adminClient.createTeam({ - name: `abac-dl-${suffix}`, - display_name: `ABAC-DL ${suffix}`, - type: 'O', - } as any); - - await adminClient.addToTeam(sharedTeam.id, userAllowed.id); - await adminClient.addToTeam(sharedTeam.id, userDenied.id); - - const channel = await createPrivateChannelForABAC(adminClient, sharedTeam.id); - await adminClient.addToChannel(userAllowed.id, channel.id); - await adminClient.addToChannel(userDenied.id, channel.id); - sharedChannelName = channel.name; - - sharedTestBrowser = new TestBrowser(browser); - - // Admin posts a file in the channel via the UI. - const {channelsPage: adminChannelsPage} = await sharedTestBrowser.login(adminUser); - await adminChannelsPage.goto(sharedTeam.name, sharedChannelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.postMessage('File attachment post', ['sample_text_file.txt']); - - // Admin opens system console, creates the attribute-based download policy. - const {systemConsolePage} = await sharedTestBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await navigateToPermissionPoliciesPage(systemConsolePage.page); - - sharedPolicyName = `Dept Download Policy ${getRandomId()}`; - await createPermissionPolicy(systemConsolePage.page, { - name: sharedPolicyName, - celExpression: 'user.attributes.Department == "Engineering"', - permissions: ['Download Files'], - adminClient: sharedAdminClient, - }); - }); - - test.afterAll(async () => { - if (sharedPolicyName && sharedAdminClient) { - await deletePermissionPolicyByName(sharedAdminClient, sharedPolicyName).catch(() => {}); - } - await sharedTestBrowser?.close().catch(() => {}); - }); - - test('MM-T5826_a user without matching attribute is denied download (Sales → placeholder)', async ({pw}) => { - test.setTimeout(60000); - test.skip(!licensed, 'No ABAC license'); - - const {page, channelsPage} = await pw.testBrowser.login(userDenied as any); - await channelsPage.goto(sharedTeam.name, sharedChannelName); - await channelsPage.toBeVisible(); - await expect - .poll(() => page.getByTestId('redactedFilesPlaceholder').isVisible(), { - timeout: 45000, - intervals: [500, 1500, 3000], - }) - .toBe(true); - await expect(page.locator('[data-testid="fileAttachmentList"]')).not.toBeVisible(); - }); - - test('MM-T5826_b user with matching attribute is granted download (Engineering → file visible)', async ({pw}) => { - test.setTimeout(60000); - test.skip(!licensed, 'No ABAC license'); - - const {page, channelsPage} = await pw.testBrowser.login(userAllowed as any); - await channelsPage.goto(sharedTeam.name, sharedChannelName); - await channelsPage.toBeVisible(); - await expect - .poll(() => page.locator('[data-testid="fileAttachmentList"]').isVisible(), { - timeout: 45000, - intervals: [500, 1500, 3000], - }) - .toBe(true); - await expect(page.getByTestId('redactedFilesPlaceholder')).not.toBeVisible(); - }); -}); - -// ─── Burn-on-Read and Permalink Edge Cases ──────────────────────────────────── - -test.describe('ABAC Permission Policies - BOR and Permalink', () => { - let lastPolicyName = ''; - let savedAdminClient: any = null; - - test.afterEach(async () => { - if (lastPolicyName && savedAdminClient) { - await deletePermissionPolicyByName(savedAdminClient, lastPolicyName); - lastPolicyName = ''; - savedAdminClient = null; - } - }); - - test('MM-T5827 denied user reveals BOR message with attachment and sees redacted placeholder', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); - - // # Admin sends a Burn-on-Read message with a file attachment - const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); - await adminChannelsPage.goto(team.name, channelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.toggleBurnOnRead(); - await adminChannelsPage.centerView.postCreate.postMessage('BOR with file', ['sample_text_file.txt']); - - // # Enable ABAC and create a policy that unconditionally denies download - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await navigateToPermissionPoliciesPage(systemConsolePage.page); - - lastPolicyName = `BOR Download Deny ${pw.random.id()}`; - await createPermissionPolicy(systemConsolePage.page, { - name: lastPolicyName, - celExpression: 'false', - permissions: ['Download Files'], - adminClient, - }); - - // Re-apply ABAC guard: a concurrent initSetup() may have reset - // AccessControlSettings.EnableAttributeBasedAccessControl to false between - // enableABAC() above and the denied user's login, preventing enforcement. - await adminClient.patchConfig({ - AccessControlSettings: {EnableAttributeBasedAccessControl: true}, - } as any); - await expect - .poll( - async () => { - const cfg = await adminClient.getConfig(); - return cfg.AccessControlSettings?.EnableAttributeBasedAccessControl === true; - }, - {timeout: 15000, intervals: [500, 1000, 2000]}, - ) - .toBe(true); - - // # Denied user navigates to channel and reveals the BOR message - const {channelsPage: deniedChannelsPage, page: deniedPage} = await pw.testBrowser.login(deniedUser); - await deniedChannelsPage.goto(team.name, channelName); - await deniedChannelsPage.toBeVisible(); - - const concealedPlaceholder = deniedPage.locator('.BurnOnReadConcealedPlaceholder').first(); - await expect(concealedPlaceholder).toBeVisible({timeout: 15000}); - await concealedPlaceholder.click(); - - // Confirm the reveal modal if it appears - const confirmModal = deniedPage.locator('.BurnOnReadConfirmationModal'); - if (await confirmModal.isVisible({timeout: 3000}).catch(() => false)) { - await confirmModal.getByRole('button', {name: /reveal/i}).click(); - } - - // * After reveal the API applies ABAC sanitization — placeholder shown, no file card - await expect(deniedPage.getByTestId('redactedFilesPlaceholder')).toBeVisible({timeout: 15000}); - await expect(deniedPage.locator('[data-testid="fileAttachmentList"]')).not.toBeVisible(); - }); - - test('MM-T5828 denied user sees no file preview inside a permalink to a post with attachment', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser: deniedUser, channelName, channelId} = await setupUserAndChannel(adminClient, team); - - // # Admin posts a message with a file attachment - const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); - await adminChannelsPage.goto(team.name, channelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.postMessage('original with file', ['sample_text_file.txt']); - - // # Retrieve the post ID and construct the permalink URL - const postsResult = await adminClient.getPosts(channelId, 0, 1); - const postId = postsResult.order[0]; - // Use testConfig.internalBaseURL, not adminClient's host-mapped route — the server itself - // must recognize this URL as its own SiteURL to embed it via an internal permalink lookup, - // instead of trying (and, in `testcontainers` mode, failing) to fetch it back over HTTP as a link. - const permalinkUrl = `${testConfig.internalBaseURL}/${team.name}/pl/${postId}`; - - // # Admin posts the permalink in the same channel (creates an embedded preview) - await adminChannelsPage.centerView.postCreate.postMessage(permalinkUrl); - - // # Enable ABAC and create a policy that unconditionally denies download - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await navigateToPermissionPoliciesPage(systemConsolePage.page); - - lastPolicyName = `Permalink Download Deny ${pw.random.id()}`; - await createPermissionPolicy(systemConsolePage.page, { - adminClient, - name: lastPolicyName, - celExpression: 'false', - permissions: ['Download Files'], - }); - - // Re-apply ABAC guard: a concurrent initSetup() may have reset - // AccessControlSettings.EnableAttributeBasedAccessControl to false between - // enableABAC() above and the denied user's login, preventing enforcement. - await adminClient.patchConfig({ - AccessControlSettings: {EnableAttributeBasedAccessControl: true}, - } as any); - await expect - .poll( - async () => { - const cfg = await adminClient.getConfig(); - return cfg.AccessControlSettings?.EnableAttributeBasedAccessControl === true; - }, - {timeout: 15000, intervals: [500, 1000, 2000]}, - ) - .toBe(true); - - // # Denied user loads the channel - const {channelsPage: deniedChannelsPage, page: deniedPage} = await pw.testBrowser.login(deniedUser); - await deniedChannelsPage.goto(team.name, channelName); - await deniedChannelsPage.toBeVisible(); - - // * The standalone original post shows the placeholder (files redacted at top level) - await expect(deniedPage.getByTestId('redactedFilesPlaceholder').first()).toBeVisible({timeout: 15000}); - - // * The embedded permalink preview (.post-preview) must ALSO show the placeholder — - // this specifically validates that the fix strips files from the embedded post - // and sets RedactedFileCount, causing the placeholder to render inside the embed. - const permalinkEmbed = deniedPage.locator('.post-preview'); - await expect(permalinkEmbed).toBeVisible({timeout: 15000}); - await expect(permalinkEmbed.getByTestId('redactedFilesPlaceholder')).toBeVisible({timeout: 10000}); - - // * No file attachment card anywhere in the channel view - await expect(deniedPage.locator('[data-testid="fileAttachmentList"]')).not.toBeVisible(); - }); -}); diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_edge_cases.spec.ts b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_edge_cases.spec.ts new file mode 100644 index 000000000000..55d3146872bd --- /dev/null +++ b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_edge_cases.spec.ts @@ -0,0 +1,246 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {expect, test, enableABAC, expectFilesRedacted, getRandomId, testConfig} from '@mattermost/playwright-lib'; + +import {createPermissionPolicy, deletePermissionPolicyByName, navigateToPermissionPoliciesPage} from '../support'; + +import {ensureABACEnabled, setupUserAndChannel, waitForPolicy} from './helpers'; + +// Each surface renders attachments through a different React tree, so one that forgets to +// honour RedactedFileCount is invisible to server-side coverage. +// +// Denies via a comparison no test user satisfies: policy creation rejects a bare `false` +// literal, and these users have no Department value. +const DENY_ALL_CEL = "user.attributes.Department == 'no-such-value-deny-all'"; +const THREAD_ROOT_MESSAGE = 'thread root with file'; + +test.describe('ABAC file permissions - redaction across surfaces', () => { + let lastPolicyName = ''; + let savedAdminClient: any = null; + + test.beforeEach(async ({pw}) => { + await pw.skipIfNoLicense(); + }); + + test.afterEach(async () => { + if (lastPolicyName && savedAdminClient) { + await deletePermissionPolicyByName(savedAdminClient, lastPolicyName); + lastPolicyName = ''; + savedAdminClient = null; + } + }); + + /** + * @objective Verify a Burn-on-Read message's attachment is redacted after the recipient + * reveals it, so the reveal cannot be used to sidestep the download policy. + */ + test( + 'MM-T5827 redacts the attachment of a revealed Burn-on-Read message', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.four_min); + + const {adminUser, adminClient, team} = await pw.initSetup(); + savedAdminClient = adminClient; + const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); + + // # Admin sends a Burn-on-Read message carrying a file + const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.toggleBurnOnRead(); + await adminChannelsPage.centerView.postCreate.postMessage('BOR with file', ['sample_text_file.txt']); + + // # Enable ABAC and deny download for everyone + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await navigateToPermissionPoliciesPage(systemConsolePage.page); + + lastPolicyName = `BOR Download Deny ${getRandomId()}`; + await createPermissionPolicy(systemConsolePage.page, { + name: lastPolicyName, + celExpression: DENY_ALL_CEL, + permissions: ['Download Files'], + adminClient, + }); + await waitForPolicy(adminClient, lastPolicyName); + await ensureABACEnabled(adminClient); + + // # The denied user opens the channel and reveals the concealed message + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + const post = await channelsPage.centerView.getLastPost(); + await post.concealedPlaceholder.toBeVisible(); + await post.concealedPlaceholder.clickToReveal(); + await post.concealedPlaceholder.waitForReveal(pw.duration.ten_sec); + + // * Verify the revealed message's file is redacted + await post.toHaveFilesRedacted(); + }, + ); + + /** + * @objective Verify the file inside an embedded permalink preview is redacted, not just the + * file on the original post — the preview is built from a separately fetched post. + */ + test( + 'MM-T5828 redacts the attachment inside an embedded permalink preview', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.four_min); + + const {adminUser, adminClient, team} = await pw.initSetup(); + savedAdminClient = adminClient; + const {testUser: deniedUser, channelName, channelId} = await setupUserAndChannel(adminClient, team); + + // # Admin posts a file, then posts a permalink to it in the same channel + const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.postMessage('original with file', ['sample_text_file.txt']); + + const originalPosts = await adminClient.getPosts(channelId, 0, 1); + const originalPostId = originalPosts.order[0]; + + // Use testConfig.internalBaseURL rather than the admin client's host-mapped route: + // the server must recognize this URL as its own SiteURL to embed it through an + // internal permalink lookup, instead of fetching it back over HTTP as a link + // (which fails under testcontainers). + const permalinkUrl = `${testConfig.internalBaseURL}/${team.name}/pl/${originalPostId}`; + await adminChannelsPage.centerView.postCreate.postMessage(permalinkUrl); + + // Both posts are addressed by ID: the embed repeats the original's text, so filtering + // by text cannot tell the two of them apart. + const permalinkPosts = await adminClient.getPosts(channelId, 0, 1); + const permalinkPostId = permalinkPosts.order[0]; + + // # Enable ABAC and deny download for everyone + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await navigateToPermissionPoliciesPage(systemConsolePage.page); + + lastPolicyName = `Permalink Download Deny ${getRandomId()}`; + await createPermissionPolicy(systemConsolePage.page, { + name: lastPolicyName, + celExpression: DENY_ALL_CEL, + permissions: ['Download Files'], + adminClient, + }); + await waitForPolicy(adminClient, lastPolicyName); + await ensureABACEnabled(adminClient); + + // # The denied user opens the channel + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the standalone original post is redacted + const originalPost = await channelsPage.centerView.getPostById(originalPostId); + await originalPost.toHaveFilesRedacted(); + + // * Verify the embedded preview is redacted too, and not merely file-less + const permalinkPost = await channelsPage.centerView.getPostById(permalinkPostId); + await expect(permalinkPost.postPreview).toBeVisible(); + await permalinkPost.toHaveFilesRedacted(permalinkPost.postPreview); + }, + ); + + /** + * @objective Verify a denied user sees the redacted placeholder when the same post is opened + * in a thread in the right-hand sidebar, which renders through its own post list. + */ + test( + 'redacts the attachment when the post is opened in a thread', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.four_min); + + const {adminUser, adminClient, team} = await pw.initSetup(); + savedAdminClient = adminClient; + const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); + + // # Admin posts a file + const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.postMessage(THREAD_ROOT_MESSAGE, ['sample_text_file.txt']); + + // # Enable ABAC and deny download for everyone + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await navigateToPermissionPoliciesPage(systemConsolePage.page); + + lastPolicyName = `Thread Download Deny ${getRandomId()}`; + await createPermissionPolicy(systemConsolePage.page, { + name: lastPolicyName, + celExpression: DENY_ALL_CEL, + permissions: ['Download Files'], + adminClient, + }); + await waitForPolicy(adminClient, lastPolicyName); + await ensureABACEnabled(adminClient); + + // # The denied user opens the post in a thread + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + const centerPost = await channelsPage.centerView.getPostByText(THREAD_ROOT_MESSAGE); + await centerPost.toHaveFilesRedacted(); + await centerPost.openAThread(); + await channelsPage.sidebarRight.toBeVisible(); + + // * Verify the thread's copy of the post is redacted as well + const rhsPost = await channelsPage.sidebarRight.getFirstPost(); + await rhsPost.toHaveFilesRedacted(); + }, + ); + + /** + * @objective Verify a denied user sees the redacted placeholder in search results, which + * render posts through the search panel rather than a channel post list. + */ + test('redacts the attachment in search results', {tag: '@abac_file_permissions'}, async ({pw}) => { + test.setTimeout(pw.duration.four_min); + + const {adminUser, adminClient, team} = await pw.initSetup(); + savedAdminClient = adminClient; + const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); + + // # Admin posts a file with a searchable, unique term + const searchTerm = `redactme${getRandomId()}`; + const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.postMessage(searchTerm, ['sample_text_file.txt']); + + // # Enable ABAC and deny download for everyone + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await navigateToPermissionPoliciesPage(systemConsolePage.page); + + lastPolicyName = `Search Download Deny ${getRandomId()}`; + await createPermissionPolicy(systemConsolePage.page, { + name: lastPolicyName, + celExpression: DENY_ALL_CEL, + permissions: ['Download Files'], + adminClient, + }); + await waitForPolicy(adminClient, lastPolicyName); + await ensureABACEnabled(adminClient); + + // # The denied user searches for the post + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + await channelsPage.searchFor(searchTerm); + + // * Verify the search result shows the redacted placeholder, not the file + const result = channelsPage.searchResultsPanel.getResultByText(searchTerm).first(); + await expect(result).toBeVisible(); + await expectFilesRedacted(result); + }); +}); diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_live_updates.spec.ts b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_live_updates.spec.ts new file mode 100644 index 000000000000..bbabc7739315 --- /dev/null +++ b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_live_updates.spec.ts @@ -0,0 +1,293 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {Client4} from '@mattermost/client'; + +import {expect, test, enableABAC, getAdminClient, getRandomId, TestBrowser} from '@mattermost/playwright-lib'; + +import type {CustomProfileAttribute} from '../../../channels/custom_profile_attributes/helpers'; +import { + setupCustomProfileAttributeFields, + setupCustomProfileAttributeValuesForUser, +} from '../../../channels/custom_profile_attributes/helpers'; +import { + createPermissionPolicy, + createPrivateChannelForABAC, + createUserForABAC, + deletePermissionPolicyByName, + enableUserManagedAttributes, + navigateToPermissionPoliciesPage, + updatePermissionPolicyExpression, +} from '../support'; + +import {ensureABACEnabled, waitForPolicy} from './helpers'; + +// Each test adds a member, and each addition appends a join system message after the fixture +// post, so the post under test is never the last one. +const FIXTURE_MESSAGE = 'File for live tests'; + +const GRANTING_EXPRESSION = 'user.attributes.Department == "Engineering"'; +const REVOKING_EXPRESSION = 'user.attributes.Department == "Finance"'; + +// Access changes reaching an already-open page. Two triggers, two websocket events: an +// attribute change sends custom_profile_attributes_values_updated, a policy edit sends +// permission_policy_updated. Each test creates its own user so a revoke cannot leak. +test.describe('ABAC file permissions - live updates', () => { + let sharedAdminClient: Client4; + let policyName = ''; + let team: any; + let channelName = ''; + let channelId = ''; + let attributeFieldsMap: Record; + let licensed = true; + let sharedBrowser: TestBrowser | null = null; + + test.beforeAll(async ({browser}) => { + test.setTimeout(240000); + + const {adminClient, adminUser} = await getAdminClient(); + if (!adminUser) { + throw new Error('Admin user not found — cannot proceed with ABAC file-access tests'); + } + sharedAdminClient = adminClient; + + try { + const lic = await adminClient.getClientLicenseOld(); + if (!lic || lic.IsLicensed !== 'true') { + licensed = false; + return; + } + } catch { + licensed = false; + return; + } + + await enableUserManagedAttributes(adminClient); + const departmentAttr: CustomProfileAttribute[] = [{name: 'Department', type: 'text', value: ''}]; + attributeFieldsMap = await setupCustomProfileAttributeFields(adminClient, departmentAttr); + + const suffix = getRandomId(); + team = await adminClient.createTeam({ + name: `abac-live-${suffix}`, + display_name: `ABAC Live ${suffix}`, + type: 'O', + } as any); + + const channel = await createPrivateChannelForABAC(adminClient, team.id); + channelName = channel.name; + channelId = channel.id; + + sharedBrowser = new TestBrowser(browser); + + // ABAC off while the admin posts the fixture file, so a policy left behind by an + // earlier spec cannot block the upload this whole describe depends on. + await adminClient.patchConfig({ + AccessControlSettings: {EnableAttributeBasedAccessControl: false}, + } as any); + + const {channelsPage: adminChannelsPage} = await sharedBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.postMessage(FIXTURE_MESSAGE, ['sample_text_file.txt']); + + const {systemConsolePage} = await sharedBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await navigateToPermissionPoliciesPage(systemConsolePage.page); + + policyName = `Dept Live Policy ${getRandomId()}`; + await createPermissionPolicy(systemConsolePage.page, { + name: policyName, + celExpression: GRANTING_EXPRESSION, + permissions: ['Upload Files', 'Download Files'], + adminClient, + }); + await waitForPolicy(adminClient, policyName); + }); + + test.beforeEach(async () => { + test.skip(!licensed, 'No ABAC license'); + await ensureABACEnabled(sharedAdminClient); + }); + + test.afterAll(async () => { + if (policyName && sharedAdminClient) { + await deletePermissionPolicyByName(sharedAdminClient, policyName).catch(() => {}); + } + await sharedBrowser?.close().catch(() => {}); + }); + + /** + * Create a user who currently satisfies the policy and drop them into the fixture channel. + * Each test gets its own so that revoking access is not visible to the others. + */ + async function createAllowedMember() { + const user = await createUserForABAC(sharedAdminClient, attributeFieldsMap, [ + {name: 'Department', type: 'text', value: 'Engineering'}, + ]); + await sharedAdminClient.addToTeam(team.id, user.id); + await sharedAdminClient.addToChannel(user.id, channelId); + return user; + } + + /** Move a user out of the department the policy grants access to. */ + async function revokeByAttribute(userId: string) { + await setupCustomProfileAttributeValuesForUser( + sharedAdminClient, + [{name: 'Department', type: 'text', value: 'Sales'}], + attributeFieldsMap, + userId, + ); + } + + /** + * @objective Verify the upload control flips from enabled to disabled on an open page when + * the user's attributes stop satisfying the policy, without a reload. + * + * @precondition + * A licensed server with a permission policy granting Upload Files to Department == "Engineering" + */ + test( + 'disables the upload control on an open page when the user attribute changes', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # A user who currently matches the policy opens the channel + const user = await createAllowedMember(); + const {channelsPage} = await pw.testBrowser.login(user); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify upload starts out available + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeEnabled({ + timeout: pw.duration.half_min, + }); + + // # Admin moves the user out of the granted department + await revokeByAttribute(user.id); + + // * Verify the control becomes unavailable without the page being reloaded + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeDisabled({ + timeout: pw.duration.half_min, + }); + }, + ); + + /** + * @objective Verify file attachments already rendered on screen are replaced by the redacted + * placeholder when the user's attributes stop satisfying the policy, without a reload. + * + * @precondition + * A licensed server with a permission policy granting Download Files to Department == "Engineering" + */ + test( + 'redacts files already on screen when the user attribute changes', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # A user who currently matches the policy opens the channel + const user = await createAllowedMember(); + const {channelsPage} = await pw.testBrowser.login(user); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the file starts out visible + const post = await channelsPage.centerView.getPostByText(FIXTURE_MESSAGE); + await post.toHaveFilesVisible(); + + // # Admin moves the user out of the granted department + await revokeByAttribute(user.id); + + // * Verify the already-loaded post is re-rendered as redacted, with no reload + await post.toHaveFilesRedacted(); + }, + ); + + /** + * @objective Verify a revoked user stays revoked across a reload — the post-list ETag epoch + * moved, so the refetch cannot be served from a cache populated while access was allowed. + * + * @precondition + * A licensed server with a permission policy granting both file permissions to Department == "Engineering" + */ + test( + 'keeps files redacted and upload blocked after a page reload', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # A user who currently matches the policy opens the channel + const user = await createAllowedMember(); + const {channelsPage, page} = await pw.testBrowser.login(user); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify access starts out granted + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeEnabled({ + timeout: pw.duration.half_min, + }); + const post = await channelsPage.centerView.getPostByText(FIXTURE_MESSAGE); + await post.toHaveFilesVisible(); + + // # Admin revokes access, then the user reloads + await revokeByAttribute(user.id); + await page.reload(); + await channelsPage.toBeVisible(); + + // * Verify the fresh fetch is sanitized rather than served from the stale cache + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeDisabled({ + timeout: pw.duration.half_min, + }); + const reloadedPost = await channelsPage.centerView.getPostByText(FIXTURE_MESSAGE); + await reloadedPost.toHaveFilesRedacted(); + }, + ); + + /** + * @objective Verify editing the policy itself — rather than the user's attributes — reaches + * an already-open page and withdraws the upload affordance. + * + * Distinct path from the tests above: a policy edit is broadcast system-wide as + * permission_policy_updated, with no per-user event to key off. + * + * Revokes by narrowing the rule's expression, not by removing the action. An action that no + * rule grants is implicitly allowed, so dropping it from the policy would widen access. + * + * @precondition + * A licensed server with a permission policy granting both file permissions to Department == "Engineering" + */ + test( + 'disables the upload control on an open page when the policy stops matching the user', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # A user who matches the policy opens the channel + const user = await createAllowedMember(); + const {channelsPage} = await pw.testBrowser.login(user); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify upload starts out available + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeEnabled({ + timeout: pw.duration.half_min, + }); + + // # Restore the fixture even if the assertion below fails, so the shared policy does + // not leak a revoking expression into the tests that run after this one. + try { + // # Admin repoints the policy at a department the user is not in, leaving the + // user's own attributes untouched + await updatePermissionPolicyExpression(sharedAdminClient, policyName, REVOKING_EXPRESSION); + + // * Verify the upload affordance is withdrawn without the page being reloaded + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeDisabled({ + timeout: pw.duration.half_min, + }); + } finally { + await updatePermissionPolicyExpression(sharedAdminClient, policyName, GRANTING_EXPRESSION); + } + }, + ); +}); diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_render.spec.ts b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_render.spec.ts new file mode 100644 index 000000000000..ce799723ceeb --- /dev/null +++ b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_render.spec.ts @@ -0,0 +1,365 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {Client4} from '@mattermost/client'; + +import {expect, test, enableABAC, getAdminClient, getRandomId, TestBrowser} from '@mattermost/playwright-lib'; + +import type {CustomProfileAttribute} from '../../../channels/custom_profile_attributes/helpers'; +import {setupCustomProfileAttributeFields} from '../../../channels/custom_profile_attributes/helpers'; +import { + cleanupAllPermissionPolicies, + createPermissionPolicy, + createPrivateChannelForABAC, + createUserForABAC, + deletePermissionPolicyByName, + enableUserManagedAttributes, + navigateToPermissionPoliciesPage, +} from '../support'; + +import {ensureABACEnabled, setupUserAndChannel, waitForPolicy} from './helpers'; + +// Targeted by text, not position: joining a channel appends a system message and later tests +// append their own posts, so the fixture post is not the last one. +const FIXTURE_MESSAGE = 'File for render tests'; +const NO_POLICY_MESSAGE = 'File attachment post'; + +// Load-time assertions only. Changes to an already-open page live in +// file_permissions_live_updates.spec.ts; non-center-channel surfaces in +// file_permissions_edge_cases.spec.ts. +test.describe('ABAC file permissions - attribute-based policy', () => { + let sharedAdminClient: Client4; + let policyName = ''; + let team: any; + let channelName = ''; + let allowedUser: any; + let deniedUser: any; + let licensed = true; + let sharedBrowser: TestBrowser | null = null; + + test.beforeAll(async ({browser}) => { + test.setTimeout(240000); + + const {adminClient, adminUser} = await getAdminClient(); + if (!adminUser) { + throw new Error('Admin user not found — cannot proceed with ABAC file-access tests'); + } + sharedAdminClient = adminClient; + + try { + const lic = await adminClient.getClientLicenseOld(); + if (!lic || lic.IsLicensed !== 'true') { + licensed = false; + return; + } + } catch { + licensed = false; + return; + } + + await enableUserManagedAttributes(adminClient); + const departmentAttr: CustomProfileAttribute[] = [{name: 'Department', type: 'text', value: ''}]; + const attributeFieldsMap = await setupCustomProfileAttributeFields(adminClient, departmentAttr); + + allowedUser = await createUserForABAC(adminClient, attributeFieldsMap, [ + {name: 'Department', type: 'text', value: 'Engineering'}, + ]); + deniedUser = await createUserForABAC(adminClient, attributeFieldsMap, [ + {name: 'Department', type: 'text', value: 'Sales'}, + ]); + + const suffix = getRandomId(); + team = await adminClient.createTeam({ + name: `abac-render-${suffix}`, + display_name: `ABAC Render ${suffix}`, + type: 'O', + } as any); + await adminClient.addToTeam(team.id, allowedUser.id); + await adminClient.addToTeam(team.id, deniedUser.id); + + const channel = await createPrivateChannelForABAC(adminClient, team.id); + channelName = channel.name; + await adminClient.addToChannel(allowedUser.id, channel.id); + await adminClient.addToChannel(deniedUser.id, channel.id); + + sharedBrowser = new TestBrowser(browser); + + // Turn ABAC off while the admin posts the fixture file, so a policy left behind by an + // earlier spec cannot block the upload this whole describe depends on. + await adminClient.patchConfig({ + AccessControlSettings: {EnableAttributeBasedAccessControl: false}, + } as any); + + const {channelsPage: adminChannelsPage} = await sharedBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.postMessage(FIXTURE_MESSAGE, ['sample_text_file.txt']); + + const {systemConsolePage} = await sharedBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await navigateToPermissionPoliciesPage(systemConsolePage.page); + + policyName = `Dept Render Policy ${getRandomId()}`; + await createPermissionPolicy(systemConsolePage.page, { + name: policyName, + celExpression: 'user.attributes.Department == "Engineering"', + permissions: ['Upload Files', 'Download Files'], + adminClient, + }); + await waitForPolicy(adminClient, policyName); + }); + + test.beforeEach(async () => { + test.skip(!licensed, 'No ABAC license'); + await ensureABACEnabled(sharedAdminClient); + }); + + test.afterAll(async () => { + if (policyName && sharedAdminClient) { + await deletePermissionPolicyByName(sharedAdminClient, policyName).catch(() => {}); + } + await sharedBrowser?.close().catch(() => {}); + }); + + /** + * @objective Verify a user whose attributes satisfy the policy sees real file attachments. + * + * @precondition + * A licensed server with a permission policy granting Download Files to Department == "Engineering" + */ + test( + 'MM-T5826_b shows file attachments to a user whose attributes match the policy', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.one_min); + + // # Log in as the user whose Department matches the policy + const {channelsPage} = await pw.testBrowser.login(allowedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the post renders its real attachments, not the redacted placeholder + const post = await channelsPage.centerView.getPostByText(FIXTURE_MESSAGE); + await post.toHaveFilesVisible(); + }, + ); + + /** + * @objective Verify a user whose attributes fail the policy sees the redacted placeholder + * instead of the file. + * + * @precondition + * A licensed server with a permission policy granting Download Files to Department == "Engineering" + */ + test( + 'MM-T5826_a redacts file attachments for a user whose attributes fail the policy', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.one_min); + + // # Log in as the user whose Department does not match the policy + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the file is replaced by the redacted placeholder + const post = await channelsPage.centerView.getPostByText(FIXTURE_MESSAGE); + await post.toHaveFilesRedacted(); + await expect(post.redactedFilesPlaceholder).toContainText('Files not available'); + }, + ); + + /** + * @objective Verify the upload affordance is offered to a user whose attributes satisfy the policy. + * + * @precondition + * A licensed server with a permission policy granting Upload Files to Department == "Engineering" + */ + test( + 'enables the upload control for a user whose attributes match the policy', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.one_min); + + // # Log in as the user whose Department matches the policy + const {channelsPage} = await pw.testBrowser.login(allowedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the attachment control is offered + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeEnabled({ + timeout: pw.duration.half_min, + }); + }, + ); + + /** + * @objective Verify the upload affordance is disabled rather than hidden for a user whose + * attributes fail the policy, so the restriction is visible instead of surfacing as a + * server error after the fact. + * + * @precondition + * A licensed server with a permission policy granting Upload Files to Department == "Engineering" + */ + test( + 'MM-T5822 disables the upload control for a user whose attributes fail the policy', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.one_min); + + // # Log in as the user whose Department does not match the policy + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the attachment control stays visible but is not actionable + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeVisible({ + timeout: pw.duration.half_min, + }); + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeDisabled(); + }, + ); + + /** + * @objective Verify a single denied user is restricted on both file actions at once — files + * redacted and upload blocked in the same view. + * + * @precondition + * A licensed server with a permission policy granting both file permissions to Department == "Engineering" + */ + test( + 'MM-T5824 redacts files and blocks upload together for a denied user', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.one_min); + + // # Log in as the user whose Department does not match the policy + const {channelsPage} = await pw.testBrowser.login(deniedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify existing files are redacted + const post = await channelsPage.centerView.getPostByText(FIXTURE_MESSAGE); + await post.toHaveFilesRedacted(); + + // * Verify new uploads are not offered + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeDisabled({ + timeout: pw.duration.half_min, + }); + }, + ); + + /** + * @objective Verify a rendered "allowed" is backed by live enforcement — the user the UI + * offers upload to can actually complete one, so the affordance and the server agree. + * + * @precondition + * A licensed server with a permission policy granting Upload Files to Department == "Engineering" + */ + test( + 'accepts a real upload from a user the render decision allows', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # Log in as the user whose Department matches the policy + const {channelsPage} = await pw.testBrowser.login(allowedUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + await expect(channelsPage.centerView.postCreate.attachmentButton).toBeEnabled({ + timeout: pw.duration.half_min, + }); + + // # Upload a file + const message = `Upload from allowed user ${getRandomId()}`; + await channelsPage.centerView.postCreate.postMessage(message, ['sample_text_file.txt']); + + // * Verify the post was accepted with its attachment intact + const post = await channelsPage.centerView.getPostByText(message); + await post.toContainText(message); + await post.toHaveFilesVisible(); + }, + ); +}); + +/** + * With ABAC enabled but no permission policy in existence, every file action is implicitly + * allowed. Separate fixture from the describe above because it needs the policy table empty. + */ +test.describe('ABAC file permissions - implicit allow with no policy', () => { + test.beforeEach(async ({pw}) => { + await pw.skipIfNoLicense(); + }); + + /** + * @objective Verify existing file attachments render normally when no permission policy + * restricts the user. + */ + test( + 'MM-T5821 shows file attachments when no permission policy exists', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # Set up a team, a private channel, and a member of it + const {adminUser, adminClient, team} = await pw.initSetup(); + const {testUser, channelName} = await setupUserAndChannel(adminClient, team); + await cleanupAllPermissionPolicies(adminClient); + + // # Admin posts a file into the channel + const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); + await adminChannelsPage.goto(team.name, channelName); + await adminChannelsPage.toBeVisible(); + await adminChannelsPage.centerView.postCreate.postMessage(NO_POLICY_MESSAGE, ['sample_text_file.txt']); + + // # Enable ABAC without creating any policy + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await ensureABACEnabled(adminClient); + + // # The member opens the channel + const {channelsPage} = await pw.testBrowser.login(testUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + // * Verify the file renders rather than being redacted + const post = await channelsPage.centerView.getPostByText(NO_POLICY_MESSAGE); + await post.toHaveFilesVisible(); + }, + ); + + /** + * @objective Verify a user can attach and send a file when no permission policy restricts + * uploads. + */ + test( + 'MM-T5823 accepts a file upload when no permission policy exists', + {tag: '@abac_file_permissions'}, + async ({pw}) => { + test.setTimeout(pw.duration.two_min); + + // # Set up a team, a private channel, and a member of it + const {adminUser, adminClient, team} = await pw.initSetup(); + const {testUser, channelName} = await setupUserAndChannel(adminClient, team); + await cleanupAllPermissionPolicies(adminClient); + + // # Enable ABAC without creating any policy + const {systemConsolePage} = await pw.testBrowser.login(adminUser); + await enableABAC(systemConsolePage.page); + await ensureABACEnabled(adminClient); + + // # The member uploads a file + const {channelsPage} = await pw.testBrowser.login(testUser); + await channelsPage.goto(team.name, channelName); + await channelsPage.toBeVisible(); + + const message = `Upload test ${getRandomId()}`; + await channelsPage.centerView.postCreate.postMessage(message, ['sample_text_file.txt']); + + // * Verify the post was accepted with its attachment intact + const post = await channelsPage.centerView.getPostByText(message); + await post.toContainText(message); + await post.toHaveFilesVisible(); + }, + ); +}); diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_upload_combined.spec.ts b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_upload_combined.spec.ts deleted file mode 100644 index 6c474f0e453b..000000000000 --- a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/file_permissions_upload_combined.spec.ts +++ /dev/null @@ -1,188 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import {expect, test, enableABAC} from '@mattermost/playwright-lib'; - -import {getAsset} from '../../../../../asset'; -import {createPermissionPolicy, deletePermissionPolicyByName, navigateToPermissionPoliciesPage} from '../support'; - -import {setupUserAndChannel} from './helpers'; - -test.describe('ABAC Permission Policies - Upload File Enforcement', () => { - let lastPolicyName = ''; - let savedAdminClient: any = null; - - test.afterEach(async () => { - if (lastPolicyName && savedAdminClient) { - await deletePermissionPolicyByName(savedAdminClient, lastPolicyName); - lastPolicyName = ''; - savedAdminClient = null; - } - }); - - test('MM-T5822 user denied upload sees error when attempting file attachment', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); - - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await navigateToPermissionPoliciesPage(systemConsolePage.page); - - lastPolicyName = `Upload Deny ${pw.random.id()}`; - await createPermissionPolicy(systemConsolePage.page, { - name: lastPolicyName, - celExpression: 'false', - permissions: ['Upload Files'], - }); - - // Re-apply ABAC guard: a concurrent initSetup() may have reset - // AccessControlSettings.EnableAttributeBasedAccessControl to false between - // enableABAC() above and the denied user's login, preventing enforcement. - await adminClient.patchConfig({ - AccessControlSettings: {EnableAttributeBasedAccessControl: true}, - } as any); - await expect - .poll( - async () => { - const cfg = await adminClient.getConfig(); - return cfg.AccessControlSettings?.EnableAttributeBasedAccessControl === true; - }, - {timeout: 15000, intervals: [500, 1000, 2000]}, - ) - .toBe(true); - - const {channelsPage: deniedChannelsPage, page: deniedPage} = await pw.testBrowser.login(deniedUser); - await deniedChannelsPage.goto(team.name, channelName); - await deniedChannelsPage.toBeVisible(); - - deniedPage.once('filechooser', async (fileChooser) => { - await fileChooser.setFiles(getAsset('mattermost.png')); - }); - await deniedChannelsPage.centerView.postCreate.attachmentButton.click(); - - await expect(deniedPage.getByText(/required access to upload/i)).toBeVisible({timeout: 30000}); - // error text already asserted above - }); - - test('MM-T5823 user can attach and send a file when no upload restriction policy exists', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser, channelName} = await setupUserAndChannel(adminClient, team); - - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await systemConsolePage.page.waitForTimeout(1000); - - const {channelsPage: userChannelsPage, page: userPage} = await pw.testBrowser.login(testUser); - await userChannelsPage.goto(team.name, channelName); - await userChannelsPage.toBeVisible(); - await userChannelsPage.centerView.postCreate.postMessage('Upload test', ['sample_text_file.txt']); - - await expect(userPage.getByText(/required access to upload/i)).not.toBeVisible(); - await expect(userPage.locator('[data-testid="fileAttachmentList"]').last()).toBeVisible({timeout: 15000}); - }); -}); - -test.describe('ABAC Permission Policies - Combined File Enforcement', () => { - let lastPolicyName = ''; - let savedAdminClient: any = null; - - test.afterEach(async () => { - if (lastPolicyName && savedAdminClient) { - await deletePermissionPolicyByName(savedAdminClient, lastPolicyName); - lastPolicyName = ''; - savedAdminClient = null; - } - }); - - test('MM-T5824 user denied both download and upload sees placeholder and cannot upload', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser: deniedUser, channelName} = await setupUserAndChannel(adminClient, team); - - const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); - await adminChannelsPage.goto(team.name, channelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.postMessage('File for combined test', ['sample_text_file.txt']); - - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await navigateToPermissionPoliciesPage(systemConsolePage.page); - - lastPolicyName = `Both Deny ${pw.random.id()}`; - await createPermissionPolicy(systemConsolePage.page, { - name: lastPolicyName, - celExpression: 'false', - permissions: ['Download Files', 'Upload Files'], - }); - - // Re-apply ABAC guard: a concurrent initSetup() may have reset - // AccessControlSettings.EnableAttributeBasedAccessControl to false between - // enableABAC() above and the denied user's login, preventing enforcement. - await adminClient.patchConfig({ - AccessControlSettings: {EnableAttributeBasedAccessControl: true}, - } as any); - await expect - .poll( - async () => { - const cfg = await adminClient.getConfig(); - return cfg.AccessControlSettings?.EnableAttributeBasedAccessControl === true; - }, - {timeout: 15000, intervals: [500, 1000, 2000]}, - ) - .toBe(true); - - const {channelsPage: deniedChannelsPage, page: deniedPage} = await pw.testBrowser.login(deniedUser); - await deniedChannelsPage.goto(team.name, channelName); - await deniedChannelsPage.toBeVisible(); - - await expect(deniedPage.getByTestId('redactedFilesPlaceholder')).toBeVisible({timeout: 15000}); - await expect(deniedPage.getByTestId('redactedFilesPlaceholder')).toContainText('Files not available'); - - deniedPage.once('filechooser', async (fileChooser) => { - await fileChooser.setFiles(getAsset('mattermost.png')); - }); - await deniedChannelsPage.centerView.postCreate.attachmentButton.click(); - await expect(deniedPage.getByText(/required access to upload/i)).toBeVisible({timeout: 15000}); - // error text already asserted above - }); - - test('MM-T5825 user can download and upload files when no restriction policies exist', async ({pw}) => { - test.setTimeout(180000); - await pw.skipIfNoLicense(); - - const {adminUser, adminClient, team} = await pw.initSetup(); - savedAdminClient = adminClient; - const {testUser, channelName} = await setupUserAndChannel(adminClient, team); - - const {channelsPage: adminChannelsPage} = await pw.testBrowser.login(adminUser); - await adminChannelsPage.goto(team.name, channelName); - await adminChannelsPage.toBeVisible(); - await adminChannelsPage.centerView.postCreate.postMessage('File for allowed test', ['sample_text_file.txt']); - - const {systemConsolePage} = await pw.testBrowser.login(adminUser); - await enableABAC(systemConsolePage.page); - await systemConsolePage.page.waitForTimeout(1000); - - const {channelsPage: userChannelsPage, page: userPage} = await pw.testBrowser.login(testUser); - await userChannelsPage.goto(team.name, channelName); - await userChannelsPage.toBeVisible(); - - await expect(userPage.locator('[data-testid="fileAttachmentList"]')).toBeVisible({timeout: 15000}); - await expect(userPage.getByTestId('redactedFilesPlaceholder')).not.toBeVisible(); - - await userChannelsPage.centerView.postCreate.postMessage('Upload from user', ['sample_text_file.txt']); - await expect(userPage.getByText(/required access to upload/i)).not.toBeVisible(); - await expect(userPage.locator('[data-testid="fileAttachmentList"]').last()).toBeVisible({timeout: 15000}); - }); -}); diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/helpers.ts b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/helpers.ts index efea77f9991c..50718a9b3084 100644 --- a/e2e-tests/playwright/specs/functional/system_console/abac/file_access/helpers.ts +++ b/e2e-tests/playwright/specs/functional/system_console/abac/file_access/helpers.ts @@ -1,7 +1,11 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {createPrivateChannelForABAC, ensureUserAttributes} from '../support'; +import type {Client4} from '@mattermost/client'; + +import {expect, getRandomId, newTestPassword} from '@mattermost/playwright-lib'; + +import {createPrivateChannelForABAC, ensureUserAttributes, getPolicyIdByName} from '../support'; export async function setupUserAndChannel( adminClient: any, @@ -15,14 +19,14 @@ export async function setupUserAndChannel( // CEL editor's "Switch to Advanced Mode" button is enabled in the UI. await ensureUserAttributes(adminClient, ['Department']); - const randomId = Math.random().toString(36).substring(2, 9); - const username = `user${randomId}`; + const username = `user${getRandomId()}`; + const password = newTestPassword(); const testUser = await adminClient.createUser( - {email: `${username}@example.com`, username, password: 'Passwd4Testing!'} as any, + {email: `${username}@example.com`, username, password} as any, '', '', ); - (testUser as any).password = 'Passwd4Testing!'; + (testUser as any).password = password; await adminClient.addToTeam(team.id, testUser.id); @@ -31,3 +35,33 @@ export async function setupUserAndChannel( return {testUser, channelName: channel.name, channelId: channel.id}; } + +/** + * Every initSetup() rewrites the shared server config, so an earlier spec can leave ABAC off. + * Without this the policy is created but never enforced, and the allow-side tests pass blindly. + */ +export async function ensureABACEnabled(adminClient: Client4): Promise { + await adminClient.patchConfig({ + AccessControlSettings: {EnableAttributeBasedAccessControl: true}, + } as any); + + await expect + .poll( + async () => { + const cfg = await adminClient.getConfig(); + return cfg.AccessControlSettings?.EnableAttributeBasedAccessControl === true; + }, + {timeout: 15000, intervals: [500, 1000, 2000]}, + ) + .toBe(true); +} + +/** Block until a just-created policy is readable through the search API. */ +export async function waitForPolicy(adminClient: Client4, policyName: string): Promise { + await expect + .poll(async () => Boolean(await getPolicyIdByName(adminClient, policyName, 1)), { + timeout: 30000, + intervals: [500, 1000, 2000], + }) + .toBe(true); +} diff --git a/e2e-tests/playwright/specs/functional/system_console/abac/support.ts b/e2e-tests/playwright/specs/functional/system_console/abac/support.ts index a9054ec2b264..96d2d38803d9 100644 --- a/e2e-tests/playwright/specs/functional/system_console/abac/support.ts +++ b/e2e-tests/playwright/specs/functional/system_console/abac/support.ts @@ -12,7 +12,7 @@ import type {UserProfile} from '@mattermost/types/users'; import type {Channel} from '@mattermost/types/channels'; import type {UserPropertyField} from '@mattermost/types/properties_user'; -import {newTestPassword} from '@mattermost/playwright-lib'; +import {getRandomId, newTestPassword} from '@mattermost/playwright-lib'; import type {CustomProfileAttribute} from '../../channels/custom_profile_attributes/helpers'; import {setupCustomProfileAttributeValuesForUser} from '../../channels/custom_profile_attributes/helpers'; @@ -195,16 +195,16 @@ export async function createUserForABAC( attributeFieldsMap: Record, attributes: CustomProfileAttribute[], ): Promise { - // Generate random ID and ensure username starts with letter - const randomId = Math.random().toString(36).substring(2, 9); - const username = `user${randomId}`.toLowerCase(); + // Username must start with a letter + const username = `user${getRandomId()}`.toLowerCase(); + const password = newTestPassword(); // Create the user const user = await adminClient.createUser( { email: `${username}@example.com`, username, - password: newTestPassword(), + password, } as any, '', '', @@ -214,7 +214,7 @@ export async function createUserForABAC( // Attach the password back to the user object so pw.testBrowser.login() can authenticate. // The API response does not include the password field. - (user as any).password = 'Passwd4Testing!'; + (user as any).password = password; return user; } @@ -1449,6 +1449,35 @@ export async function navigateToPermissionPoliciesPage(page: Page): Promise { + const policyId = await getPolicyIdByName(client, policyName); + expect(policyId, `Could not find permission policy "${policyName}" to update`).toBeTruthy(); + + const policy = await (client as any).doFetch(`${client.getBaseRoute()}/access_control_policies/${policyId}`, { + method: 'GET', + }); + + policy.rules = (policy.rules || []).map((rule: any) => ({...rule, expression})); + + await (client as any).doFetch(`${client.getBaseRoute()}/access_control_policies`, { + method: 'PUT', + body: JSON.stringify(policy), + }); +} + export async function deletePermissionPolicyByName(client: Client4, policyName: string): Promise { try { const searchUrl = `${client.getBaseRoute()}/access_control_policies/search`; diff --git a/server/channels/api4/access_control.go b/server/channels/api4/access_control.go index 3d4dd338d5f4..4b8af4339d9d 100644 --- a/server/channels/api4/access_control.go +++ b/server/channels/api4/access_control.go @@ -63,6 +63,52 @@ func (api *API) InitAccessControlPolicy() { api.BaseRoutes.AccessControlPolicy.Handle("/unassign", api.APISessionRequired(unassignAccessPolicy)).Methods(http.MethodDelete) api.BaseRoutes.AccessControlPolicy.Handle("/resources/channels", api.APISessionRequired(getChannelsForAccessControlPolicy)).Methods(http.MethodGet) api.BaseRoutes.AccessControlPolicy.Handle("/resources/channels/search", api.APISessionRequired(searchChannelsForAccessControlPolicy)).Methods(http.MethodPost) + + api.BaseRoutes.AccessControlDecisions.Handle("/actions/search", api.APISessionRequired(searchAccessControlDecisionActions)).Methods(http.MethodPost) +} + +// searchAccessControlDecisionActions returns non-authoritative, render-time ABAC +// decisions for the current session user on a single resource. If Subject is +// provided in the request it must match the authenticated session user ID — any +// other value is rejected with 403. Results are for rendering only; protected +// endpoints always re-evaluate the PDP live. +func searchAccessControlDecisionActions(c *Context, w http.ResponseWriter, r *http.Request) { + var req model.ActionSearchRequest + if jsonErr := json.NewDecoder(r.Body).Decode(&req); jsonErr != nil { + c.SetInvalidParamWithErr("action_search", jsonErr) + return + } + if appErr := req.IsValid(); appErr != nil { + c.Err = appErr + return + } + // Fail closed on resource types we can't authorize: only channel decisions are exposed + // today, so anything else is rejected rather than silently skipping the access check. + switch req.Resource.Type { + case model.AccessControlPolicyTypeChannel: + if hasPermission, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), req.Resource.ID, model.PermissionReadChannel); !hasPermission { + c.SetPermissionError(model.PermissionReadChannel) + return + } + default: + c.Err = model.NewAppError("searchAccessControlDecisionActions", "api.access_control.decision.unsupported_resource_type.app_error", map[string]any{"Type": req.Resource.Type}, "", http.StatusBadRequest) + return + } + + resp, appErr := c.App.SearchAllowedActionsForCurrentUser(c.AppContext, req) + if appErr != nil { + c.Err = appErr + return + } + + js, err := json.Marshal(resp) + if err != nil { + c.Err = model.NewAppError("searchAccessControlDecisionActions", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) + return + } + if _, err := w.Write(js); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } } func createAccessControlPolicy(c *Context, w http.ResponseWriter, r *http.Request) { diff --git a/server/channels/api4/access_control_decision_test.go b/server/channels/api4/access_control_decision_test.go new file mode 100644 index 000000000000..68ff4a8972a0 --- /dev/null +++ b/server/channels/api4/access_control_decision_test.go @@ -0,0 +1,175 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package api4 + +import ( + "context" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin/plugintest/mock" + "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" + "github.com/stretchr/testify/require" +) + +func TestSearchAccessControlDecisionActions(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.PermissionPolicies = true + }).InitBasic(t) + + channelResource := model.Resource{Type: model.AccessControlPolicyTypeChannel, ID: th.BasicChannel.Id} + + t.Run("requires a session", func(t *testing.T) { + client := th.CreateClient() // unauthenticated + _, resp, err := client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Error(t, err) + CheckUnauthorizedStatus(t, resp) + }) + + t.Run("unsupported action returns bad request", func(t *testing.T) { + _, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{"definitely_not_a_real_action"}, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("invalid request returns bad request", func(t *testing.T) { + _, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: model.Resource{Type: "", ID: th.BasicChannel.Id}, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("unsupported resource type returns bad request", func(t *testing.T) { + // A non-channel resource type must be rejected outright rather than silently skipping + // the resource-access check. Discovery mode (no actions) previously returned an empty 200. + _, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: model.Resource{Type: model.AccessControlPolicyTypeTeam, ID: th.BasicTeam.Id}, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("returns allowed when ABAC is inactive", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = false + }) + + out, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment, model.AccessControlPolicyActionDownloadFileAttachment}, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Len(t, out.Decisions, 2) + require.True(t, out.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Allowed) + require.True(t, out.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Evaluated) + }) + + t.Run("returns PDP deny for the session user", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = true + }) + mockACS := &mocks.AccessControlServiceInterface{} + original := th.App.Srv().Channels().AccessControl + th.App.Srv().Channels().AccessControl = mockACS + defer func() { th.App.Srv().Channels().AccessControl = original }() + + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Resource.ID == th.BasicChannel.Id && req.Action == model.AccessControlPolicyActionUploadFileAttachment + })).Return(model.AccessDecision{Decision: false}, (*model.AppError)(nil)) + + out, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.False(t, out.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Allowed) + require.True(t, out.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Evaluated) + require.Empty(t, out.Results) // denied actions must not appear in the AuthZEN results list + }) + + t.Run("discovery mode returns all actions when ABAC inactive", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = false + }) + + out, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Len(t, out.Decisions, 2) + require.Len(t, out.Results, 2) + }) + + t.Run("subject not matching session user returns 403", func(t *testing.T) { + _, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Subject: &model.ActionSearchSubject{ID: model.NewId()}, + }) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("subject matching session user is accepted", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = false + }) + + out, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Subject: &model.ActionSearchSubject{ID: th.BasicUser.Id}, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.True(t, out.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Allowed) + }) + + t.Run("non-member of private channel returns 403", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = false + }) + // Create a private channel as system admin; BasicUser is not a member. + privateCh, _, err := th.SystemAdminClient.CreateChannel(context.Background(), &model.Channel{ + TeamId: th.BasicTeam.Id, + Name: model.NewId(), + DisplayName: "Private Not Member", + Type: model.ChannelTypePrivate, + }) + require.NoError(t, err) + + _, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: model.Resource{Type: model.AccessControlPolicyTypeChannel, ID: privateCh.Id}, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("page field accepted and ignored", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = false + }) + + out, resp, err := th.Client.SearchAccessControlDecisionActions(context.Background(), model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Page: &model.ActionSearchPage{NextToken: "tok"}, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Nil(t, out.Page) + }) +} diff --git a/server/channels/api4/api.go b/server/channels/api4/api.go index dff1e99090a8..87ebaf7cf528 100644 --- a/server/channels/api4/api.go +++ b/server/channels/api4/api.go @@ -166,8 +166,9 @@ type Routes struct { AuditLogs *mux.Router // 'api/v4/audit_logs' - AccessControlPolicies *mux.Router // 'api/v4/access_control_policies' - AccessControlPolicy *mux.Router // 'api/v4/access_control_policies/{policy_id:[A-Za-z0-9]+}' + AccessControlPolicies *mux.Router // 'api/v4/access_control_policies' + AccessControlPolicy *mux.Router // 'api/v4/access_control_policies/{policy_id:[A-Za-z0-9]+}' + AccessControlDecisions *mux.Router // 'api/v4/access_control/decisions' ContentFlagging *mux.Router // 'api/v4/content_flagging' @@ -334,6 +335,7 @@ func Init(srv *app.Server) (*API, error) { api.BaseRoutes.AccessControlPolicies = api.BaseRoutes.APIRoot.PathPrefix("/access_control_policies").Subrouter() api.BaseRoutes.AccessControlPolicy = api.BaseRoutes.APIRoot.PathPrefix("/access_control_policies/{policy_id:[A-Za-z0-9]+}").Subrouter() + api.BaseRoutes.AccessControlDecisions = api.BaseRoutes.APIRoot.PathPrefix("/access_control/decisions").Subrouter() api.BaseRoutes.ContentFlagging = api.BaseRoutes.APIRoot.PathPrefix("/content_flagging").Subrouter() diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index c5ec618cc289..58117bffd8da 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -1119,7 +1119,8 @@ func getPinnedPosts(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.HandleEtag(posts.Etag(), "Get Pinned Posts", w, r) { + pinnedEtag := c.App.AppendABACEtag(posts.Etag(), c.AppContext.Session().UserId, c.Params.ChannelId) + if c.HandleEtag(pinnedEtag, "Get Pinned Posts", w, r) { return } @@ -1130,7 +1131,7 @@ func getPinnedPosts(c *Context, w http.ResponseWriter, r *http.Request) { return } - w.Header().Set(model.HeaderEtagServer, clientPostList.Etag()) + w.Header().Set(model.HeaderEtagServer, pinnedEtag) if err := clientPostList.EncodeJSON(w); err != nil { c.Logger.Warn("Error while writing response", mlog.Err(err)) } diff --git a/server/channels/api4/file_test.go b/server/channels/api4/file_test.go index 642fe29e7367..ae648b47134e 100644 --- a/server/channels/api4/file_test.go +++ b/server/channels/api4/file_test.go @@ -29,9 +29,11 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin/plugintest/mock" "github.com/mattermost/mattermost/server/v8/channels/app" "github.com/mattermost/mattermost/server/v8/channels/utils/fileutils" "github.com/mattermost/mattermost/server/v8/channels/utils/testutils" + "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" ) var testDir = "" @@ -1879,3 +1881,77 @@ func TestHeadRequestsFileEndpoints(t *testing.T) { require.Equal(t, http.StatusNotFound, resp.StatusCode) }) } + +func TestUploadFileABACEnforcement(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.PermissionPolicies = true + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(true) + }).InitBasic(t) + + if *th.App.Config().FileSettings.DriverName == "" { + t.Skip("skipping because no file driver is enabled") + } + + sent, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) + + // The channel permission check runs before ABAC, so the user must already be able to upload + // for this to reach the gate under test. + mockUploadDecision := func(t *testing.T, allowed bool) { + t.Helper() + + mockACS := &mocks.AccessControlServiceInterface{} + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Action == model.AccessControlPolicyActionUploadFileAttachment + })).Return(model.AccessDecision{Decision: allowed}, (*model.AppError)(nil)) + + original := th.App.Srv().Channels().AccessControl + th.App.Srv().Channels().AccessControl = mockACS + t.Cleanup(func() { + th.App.Srv().Channels().AccessControl = original + }) + } + + // UploadFileAsRequestBody drives uploadFileSimple, UploadFile drives uploadFileMultipart. + // Each carries its own copy of the gate. + t.Run("simple upload is rejected when the policy denies the upload action", func(t *testing.T) { + mockUploadDecision(t, false) + + _, resp, err := th.Client.UploadFileAsRequestBody(context.Background(), sent, th.BasicChannel.Id, "test.png") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("multipart upload is rejected when the policy denies the upload action", func(t *testing.T) { + mockUploadDecision(t, false) + + _, resp, err := th.Client.UploadFile(context.Background(), sent, th.BasicChannel.Id, "test.png") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("upload is accepted when the policy allows the upload action", func(t *testing.T) { + mockUploadDecision(t, true) + + fileResp, _, err := th.Client.UploadFile(context.Background(), sent, th.BasicChannel.Id, "test.png") + require.NoError(t, err) + require.Len(t, fileResp.FileInfos, 1) + }) + + t.Run("upload is unaffected when ABAC is disabled", func(t *testing.T) { + mockUploadDecision(t, false) + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(false) + }) + t.Cleanup(func() { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(true) + }) + }) + + fileResp, _, err := th.Client.UploadFile(context.Background(), sent, th.BasicChannel.Id, "test.png") + require.NoError(t, err) + require.Len(t, fileResp.FileInfos, 1) + }) +} diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index 36774f68962b..d0bfb9e4e2ab 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -331,7 +331,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) { if since > 0 { list, err = c.App.GetPostsSince(c.AppContext, model.GetPostsSinceOptions{ChannelId: channelId, Time: since, SkipFetchThreads: skipFetchThreads, CollapsedThreads: collapsedThreads, CollapsedThreadsExtended: collapsedThreadsExtended, UserId: c.AppContext.Session().UserId}) } else if afterPost != "" { - etag = c.App.GetPostsEtag(channelId, collapsedThreads) + etag = c.App.GetPostsEtag(channelId, c.AppContext.Session().UserId, collapsedThreads) if c.HandleEtag(etag, "Get Posts After", w, r) { return @@ -339,7 +339,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) { list, err = c.App.GetPostsAfterPost(c.AppContext, model.GetPostsOptions{ChannelId: channelId, PostId: afterPost, Page: page, PerPage: perPage, SkipFetchThreads: skipFetchThreads, CollapsedThreads: collapsedThreads, UserId: c.AppContext.Session().UserId, IncludeDeleted: includeDeleted}) } else if beforePost != "" { - etag = c.App.GetPostsEtag(channelId, collapsedThreads) + etag = c.App.GetPostsEtag(channelId, c.AppContext.Session().UserId, collapsedThreads) if c.HandleEtag(etag, "Get Posts Before", w, r) { return @@ -347,7 +347,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) { list, err = c.App.GetPostsBeforePost(c.AppContext, model.GetPostsOptions{ChannelId: channelId, PostId: beforePost, Page: page, PerPage: perPage, SkipFetchThreads: skipFetchThreads, CollapsedThreads: collapsedThreads, CollapsedThreadsExtended: collapsedThreadsExtended, UserId: c.AppContext.Session().UserId, IncludeDeleted: includeDeleted}) } else { - etag = c.App.GetPostsEtag(channelId, collapsedThreads) + etag = c.App.GetPostsEtag(channelId, c.AppContext.Session().UserId, collapsedThreads) if c.HandleEtag(etag, "Get Posts", w, r) { return @@ -433,7 +433,7 @@ func getPostsForChannelAroundLastUnread(c *Context, w http.ResponseWriter, r *ht etag := "" if len(postList.Order) == 0 { - etag = c.App.GetPostsEtag(channelId, collapsedThreads) + etag = c.App.GetPostsEtag(channelId, c.AppContext.Session().UserId, collapsedThreads) if c.HandleEtag(etag, "Get Posts", w, r) { return @@ -608,11 +608,12 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.HandleEtag(post.Etag(), "Get Post", w, r) { + postEtag := c.App.AppendABACEtag(post.Etag(), c.AppContext.Session().UserId, post.ChannelId) + if c.HandleEtag(postEtag, "Get Post", w, r) { return } - w.Header().Set(model.HeaderEtagServer, post.Etag()) + w.Header().Set(model.HeaderEtagServer, postEtag) if err := post.EncodeJSON(w); err != nil { c.Logger.Warn("Error while writing response", mlog.Err(err)) } @@ -925,7 +926,8 @@ func getPostThread(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.HandleEtag(list.Etag(), "Get Post Thread", w, r) { + threadEtag := c.App.AppendABACEtag(list.Etag(), c.AppContext.Session().UserId, post.ChannelId) + if c.HandleEtag(threadEtag, "Get Post Thread", w, r) { return } @@ -936,7 +938,7 @@ func getPostThread(c *Context, w http.ResponseWriter, r *http.Request) { return } - w.Header().Set(model.HeaderEtagServer, clientPostList.Etag()) + w.Header().Set(model.HeaderEtagServer, threadEtag) if err := clientPostList.EncodeJSON(w); err != nil { c.Logger.Warn("Error while writing response", mlog.Err(err)) diff --git a/server/channels/app/access_control.go b/server/channels/app/access_control.go index 13619fc7da2b..e704a5e41b2e 100644 --- a/server/channels/app/access_control.go +++ b/server/channels/app/access_control.go @@ -19,8 +19,34 @@ import ( ) const attributeViewRefreshInterval = 30 * time.Second + const accessControlChildPolicySearchLimit = 1000 +// attributeBasedAccessControlEnabled must stay the exact predicate the enforcement paths use, with +// no license check, so the render-ETag and cache-invalidation gates can never be narrower than the +// sanitization they keep in step with. +func attributeBasedAccessControlEnabled(cfg *model.Config) bool { + return cfg.FeatureFlags.PermissionPolicies && + cfg.AccessControlSettings.EnableAttributeBasedAccessControl != nil && + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl +} + +func (a *App) attributeBasedAccessControlEnabled() bool { + return attributeBasedAccessControlEnabled(a.Config()) +} + +// clearABACRenderCachesOnFlip drops both render-ETag epoch caches when ABAC is toggled: they are +// only invalidated while ABAC is active, so a change made while it was off would leave a stale +// epoch for the switch back on to serve. +func (ch *Channels) clearABACRenderCachesOnFlip(prevCfg, cfg *model.Config) { + if attributeBasedAccessControlEnabled(prevCfg) == attributeBasedAccessControlEnabled(cfg) { + return + } + + ch.srv.Store().AccessControlPolicy().ClearEtagCache() + ch.srv.Store().Attributes().ClearUserPropertyValuesEpochCache() +} + // ResourceAttributesInPoliciesEnabled reports whether access rules may compare a // user's attributes against the accessed channel's. It gates authoring only: the // autocomplete endpoint stops offering channel-object-type fields, so no editor @@ -207,6 +233,8 @@ func (a *App) CreateOrUpdateAccessControlPolicy(rctx request.CTX, policy *model. case model.AccessControlPolicyTypeParent: a.publishChannelPolicyEnforcedForChannelPoliciesWithImport(rctx, policy.ID) a.publishTeamPolicyEnforcedForTeamPoliciesWithImport(rctx, policy.ID) + case model.AccessControlPolicyTypePermission: + a.publishPermissionPolicyUpdate(rctx) } return policy, nil @@ -500,6 +528,8 @@ func (a *App) DeleteAccessControlPolicy(rctx request.CTX, id string) *model.AppE case policy.Type == model.AccessControlPolicyTypeParent: a.publishChannelPolicyEnforcedUpdatesForChannels(rctx, affectedChannelIDs) a.publishTeamPolicyEnforcedUpdatesForTeams(rctx, affectedTeamIDs) + case policy.Type == model.AccessControlPolicyTypePermission: + a.publishPermissionPolicyUpdate(rctx) } return nil @@ -1877,6 +1907,7 @@ func (a *App) UpdateAccessControlPoliciesActive(rctx request.CTX, updates []mode return nil, model.NewAppError("UpdateAccessControlPoliciesActive", "app.pap.update_access_control_policies_active.app_error", nil, err.Error(), http.StatusInternalServerError) } + permissionPolicyChanged := false for _, policy := range policies { switch policy.Type { case model.AccessControlPolicyTypeChannel: @@ -1886,8 +1917,13 @@ func (a *App) UpdateAccessControlPoliciesActive(rctx request.CTX, updates []mode case model.AccessControlPolicyTypeParent: a.publishChannelPolicyEnforcedForChannelPoliciesWithImport(rctx, policy.ID) a.publishTeamPolicyEnforcedForTeamPoliciesWithImport(rctx, policy.ID) + case model.AccessControlPolicyTypePermission: + permissionPolicyChanged = true } } + if permissionPolicyChanged { + a.publishPermissionPolicyUpdate(rctx) + } return policies, nil } @@ -2172,6 +2208,7 @@ func (a *App) HydrateTeamsPolicyActions(rctx request.CTX, teams []*model.Team) * // only need to refresh access control state — not run the full // channel_updated reducer/router pipeline. func (a *App) publishChannelPolicyEnforcedUpdate(rctx request.CTX, channelID string) { + a.Srv().Store().AccessControlPolicy().InvalidateEtagForChannel(channelID) a.Srv().Store().Channel().InvalidateChannel(channelID) channel, appErr := a.GetChannel(rctx, channelID) @@ -2210,6 +2247,16 @@ func (a *App) publishChannelPolicyEnforcedUpdate(rctx request.CTX, channelID str a.Publish(messageWs) } +// publishPermissionPolicyUpdate broadcasts a system-scoped permission policy change. +// TypePermission policies are global, so no channel ID is included in the event. +func (a *App) publishPermissionPolicyUpdate(rctx request.CTX) { + // Permission policies are system-scoped, so every channel's render-ETag epoch folds them in. + // Clear the whole epoch cache rather than a single channel key. + a.Srv().Store().AccessControlPolicy().ClearEtagCache() + messageWs := model.NewWebSocketEvent(model.WebsocketEventPermissionPolicyUpdated, "", "", "", nil, "") + a.Publish(messageWs) +} + // publishTeamPolicyEnforcedUpdate reloads the team, hydrates its policy // actions, and broadcasts a team_access_control_updated websocket event so // connected clients can refresh their view of the team's access control state @@ -2739,6 +2786,27 @@ func ResolveSystemRole(roles string) string { return model.SystemUserRoleId } +// invalidateAttributeViewCache forces the next refreshAttributeViewIfStale call to refresh. +// +// Deliberately not rate-limited: the REFRESH happens on the read side, so a bulk writer with no +// evaluations in between still costs one refresh, while deferring a write loses it — a client asks +// for a render decision once per change event and never re-asks. +// +// The marker is node-local and the matview shared, so in HA only the writing node gets live +// visibility; elsewhere the bound stays the periodic one until this grows a cluster message. +func (a *App) invalidateAttributeViewCache() { + ch := a.Srv().Channels() + + if ch.attributeViewRefreshMut.TryLock() { + ch.attributeViewRefreshLast = time.Time{} + ch.attributeViewRefreshMut.Unlock() + } else { + // The in-flight refresh may have read the DB before this write landed, and it will + // overwrite the timer on completion, so flag a follow-up for the next caller. + ch.attributeViewNeedsRefresh.Store(true) + } +} + // refreshAttributeViewIfStale refreshes the attribute materialized views if the // last refresh was more than attributeViewRefreshInterval ago. The refresh is // non-blocking: if another goroutine is already refreshing, this call returns @@ -2751,7 +2819,8 @@ func (a *App) refreshAttributeViewIfStale(rctx request.CTX) { } defer ch.attributeViewRefreshMut.Unlock() - if time.Since(ch.attributeViewRefreshLast) < attributeViewRefreshInterval { + needsRefresh := ch.attributeViewNeedsRefresh.Swap(false) + if !needsRefresh && time.Since(ch.attributeViewRefreshLast) < attributeViewRefreshInterval { return } diff --git a/server/channels/app/access_control_decision.go b/server/channels/app/access_control_decision.go new file mode 100644 index 000000000000..b15aa85c1084 --- /dev/null +++ b/server/channels/app/access_control_decision.go @@ -0,0 +1,168 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "net/http" + "slices" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" +) + +// renderableActionConfig controls fallback behavior when ABAC is inactive or evaluation fails. +type renderableActionConfig struct { + ResourceType string + DefaultWhenInactive bool + // FailClosedOnError returns denied+evaluated on subject-build or PDP errors + // rather than falling back to DefaultWhenInactive. Set for security-sensitive actions. + FailClosedOnError bool +} + +// renderableABACActions is the allowlist of ABAC actions that may be queried +// through the render-decision (Action Search) API. Any action not present here +// is rejected with 400 to prevent arbitrary action probing and to centralize the +// security posture for each renderable affordance. +var renderableABACActions = map[string]renderableActionConfig{ + model.AccessControlPolicyActionUploadFileAttachment: { + ResourceType: model.AccessControlPolicyTypeChannel, + DefaultWhenInactive: true, + FailClosedOnError: true, + }, + model.AccessControlPolicyActionDownloadFileAttachment: { + ResourceType: model.AccessControlPolicyTypeChannel, + DefaultWhenInactive: true, + FailClosedOnError: true, + }, +} + +// SearchAllowedActionsForCurrentUser computes non-authoritative, render-time ABAC +// decisions for the current session user on a single resource. It mirrors the +// enforcement path (BuildAccessControlSubjectForSession + AccessEvaluation with +// the same Resource shape) so a render "allowed" can never disagree with what +// enforcement would decide. Results MUST NOT be used to authorize an action; the +// protected endpoints always re-evaluate the PDP live. +// +// When req.Actions is nil/empty the function operates in discovery mode: it +// evaluates all renderable actions registered for the resource type and returns +// the permitted set. When req.Actions is non-empty only those specific actions +// are evaluated (targeted mode). +func (a *App) SearchAllowedActionsForCurrentUser(rctx request.CTX, req model.ActionSearchRequest) (*model.ActionSearchResponse, *model.AppError) { + if appErr := req.IsValid(); appErr != nil { + return nil, appErr + } + + // Subject reservation: any Subject whose ID != session user is rejected. + // This gate exists to make Phase 3 (arbitrary-subject evaluation, Enterprise) + // a non-breaking extension — the field is in the contract but gated here. + if req.Subject != nil && req.Subject.ID != rctx.Session().UserId { + return nil, model.NewAppError( + "SearchAllowedActionsForCurrentUser", + "app.access_control_decision.subject_mismatch.app_error", + nil, "", http.StatusForbidden) + } + + // Discovery mode: collect registry entries for the resource type, then sort for + // deterministic wire output — Go map iteration is non-deterministic. + // Targeted mode: validate each requested action against the registry. + var candidates []string + if len(req.Actions) == 0 { + for action, cfg := range renderableABACActions { + if cfg.ResourceType == req.Resource.Type { + candidates = append(candidates, action) + } + } + slices.Sort(candidates) + } else { + for _, action := range req.Actions { + cfg, ok := renderableABACActions[action] + if !ok || cfg.ResourceType != req.Resource.Type { + return nil, model.NewAppError("SearchAllowedActionsForCurrentUser", "app.access_control_decision.unsupported_action.app_error", map[string]any{"Action": action}, "", http.StatusBadRequest) + } + } + candidates = req.Actions + } + + resp := &model.ActionSearchResponse{ + Resource: req.Resource, + Results: []model.ActionSearchResult{}, // always non-nil so it serializes as [] + Decisions: make(map[string]model.RenderPermissionDecision, len(candidates)), // always non-nil so it serializes as {} + } + record := func(action string, d model.RenderPermissionDecision) { + resp.Decisions[action] = d + if d.Allowed { + resp.Results = append(resp.Results, model.ActionSearchResult{Action: model.ActionSearchResultAction{Name: action}}) + } + } + + // No active policy — return per-action defaults. + acs := a.Srv().Channels().AccessControl + if acs == nil || !a.attributeBasedAccessControlEnabled() { + for _, action := range candidates { + record(action, model.RenderPermissionDecision{ + Allowed: renderableABACActions[action].DefaultWhenInactive, + Evaluated: true, + }) + } + return resp, nil + } + + // All currently registered resource types are channel-scoped, so req.Resource.ID + // is always a channel ID here. If a non-channel resource type is ever added to + // renderableABACActions, this call must be updated to pass the correct channel ID. + subject, appErr := a.BuildAccessControlSubjectForSession(rctx, req.Resource.ID) + if appErr != nil { + rctx.Logger().Info("Failed to build ABAC subject for render-decision search", + mlog.String("resource_type", req.Resource.Type), + mlog.String("resource_id", req.Resource.ID), + mlog.Err(appErr), + ) + for _, action := range candidates { + record(action, renderDecisionOnError(action)) + } + return resp, nil + } + + for _, action := range candidates { + decision, evalErr := acs.AccessEvaluation(rctx, model.AccessRequest{ + Subject: *subject, + Resource: req.Resource, + Action: action, + }) + if evalErr != nil { + rctx.Logger().Debug("ABAC render-decision evaluation failed", + mlog.String("action", action), + mlog.String("resource_id", req.Resource.ID), + mlog.Err(evalErr), + ) + record(action, renderDecisionOnError(action)) + continue + } + record(action, model.RenderPermissionDecision{ + Allowed: decision.Decision, + Evaluated: true, + }) + } + + return resp, nil +} + +// renderDecisionOnError returns the conservative decision for an action whose +// subject build or PDP evaluation failed: fail closed (deny + generic reason) +// for security-sensitive actions, otherwise fall back to the inactive default. +func renderDecisionOnError(action string) model.RenderPermissionDecision { + cfg := renderableABACActions[action] + if cfg.FailClosedOnError { + return model.RenderPermissionDecision{ + Allowed: false, + Evaluated: true, + Reason: model.RenderDecisionReasonRestrictedByPolicy, + } + } + return model.RenderPermissionDecision{ + Allowed: cfg.DefaultWhenInactive, + Evaluated: true, + } +} diff --git a/server/channels/app/access_control_decision_test.go b/server/channels/app/access_control_decision_test.go new file mode 100644 index 000000000000..34876b90bdc0 --- /dev/null +++ b/server/channels/app/access_control_decision_test.go @@ -0,0 +1,313 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost/server/public/model" + eMocks "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" +) + +func TestSearchAllowedActionsForCurrentUser(t *testing.T) { + mainHelper.Parallel(t) + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.PermissionPolicies = true + }).InitBasic(t) + + // rctx carrying a real session for BasicUser, required by the + // session-subject build path. + session, appErr := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser.Id, Props: model.StringMap{}}) + require.Nil(t, appErr) + rctx := th.Context.WithSession(session) + + channelResource := model.Resource{Type: model.AccessControlPolicyTypeChannel, ID: th.BasicChannel.Id} + + enableABAC := func(t *testing.T) { + t.Helper() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = true + }) + } + disableABAC := func(t *testing.T) { + t.Helper() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = false + }) + } + + withMockACS := func(t *testing.T) *eMocks.AccessControlServiceInterface { + t.Helper() + mockACS := &eMocks.AccessControlServiceInterface{} + original := th.App.Srv().ch.AccessControl + th.App.Srv().ch.AccessControl = mockACS + t.Cleanup(func() { th.App.Srv().ch.AccessControl = original }) + return mockACS + } + + t.Run("invalid request returns bad request", func(t *testing.T) { + _, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: model.Resource{Type: "", ID: th.BasicChannel.Id}, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.NotNil(t, appErr) + require.Equal(t, 400, appErr.StatusCode) + }) + + t.Run("unsupported action returns bad request", func(t *testing.T) { + _, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{"definitely_not_a_real_action"}, + }) + require.NotNil(t, appErr) + require.Equal(t, 400, appErr.StatusCode) + }) + + t.Run("action with wrong resource type returns bad request", func(t *testing.T) { + _, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: model.Resource{Type: model.AccessControlPolicyTypeTeam, ID: th.BasicTeam.Id}, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.NotNil(t, appErr) + require.Equal(t, 400, appErr.StatusCode) + }) + + t.Run("ABAC inactive returns allowed and evaluated", func(t *testing.T) { + disableABAC(t) + original := th.App.Srv().ch.AccessControl + th.App.Srv().ch.AccessControl = nil + defer func() { th.App.Srv().ch.AccessControl = original }() + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment, model.AccessControlPolicyActionDownloadFileAttachment}, + }) + require.Nil(t, appErr) + require.Len(t, resp.Decisions, 2) + for _, action := range []string{model.AccessControlPolicyActionUploadFileAttachment, model.AccessControlPolicyActionDownloadFileAttachment} { + require.True(t, resp.Decisions[action].Evaluated, action) + require.True(t, resp.Decisions[action].Allowed, action) + require.Empty(t, resp.Decisions[action].Reason, action) + } + }) + + t.Run("ABAC allow returns allowed", func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Resource.ID == th.BasicChannel.Id && req.Action == model.AccessControlPolicyActionUploadFileAttachment + })).Return(model.AccessDecision{Decision: true}, (*model.AppError)(nil)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Nil(t, appErr) + require.True(t, resp.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Allowed) + require.True(t, resp.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Evaluated) + }) + + t.Run("ABAC deny returns not allowed", func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Resource.ID == th.BasicChannel.Id && req.Action == model.AccessControlPolicyActionUploadFileAttachment + })).Return(model.AccessDecision{Decision: false}, (*model.AppError)(nil)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Nil(t, appErr) + require.False(t, resp.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Allowed) + require.True(t, resp.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Evaluated) + }) + + t.Run("evaluation error fails closed for sensitive action", func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.Anything). + Return(model.AccessDecision{}, model.NewAppError("test", "test.error", nil, "", 500)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionDownloadFileAttachment}, + }) + require.Nil(t, appErr) + d := resp.Decisions[model.AccessControlPolicyActionDownloadFileAttachment] + require.False(t, d.Allowed) + require.True(t, d.Evaluated) + require.Equal(t, model.RenderDecisionReasonRestrictedByPolicy, d.Reason) + }) + + t.Run("builds subject once and evaluates once per action", func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Resource.ID == th.BasicChannel.Id + })).Return(model.AccessDecision{Decision: true}, (*model.AppError)(nil)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment, model.AccessControlPolicyActionDownloadFileAttachment}, + }) + require.Nil(t, appErr) + require.Len(t, resp.Decisions, 2) + mockACS.AssertNumberOfCalls(t, "AccessEvaluation", 2) + }) + + for _, want := range []bool{true, false} { + t.Run(fmt.Sprintf("render decision matches enforcement (pdp=%v)", want), func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Resource.ID == th.BasicChannel.Id && req.Action == model.AccessControlPolicyActionUploadFileAttachment + })).Return(model.AccessDecision{Decision: want}, (*model.AppError)(nil)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Nil(t, appErr) + + enforced := th.App.HasPermissionToFileAction(rctx, th.BasicUser.Id, th.BasicUser.Roles, th.BasicChannel.Id, model.AccessControlPolicyActionUploadFileAttachment) + require.Equal(t, enforced, resp.Decisions[model.AccessControlPolicyActionUploadFileAttachment].Allowed) + require.Equal(t, want, enforced) + }) + } + + // --- Discovery mode --- + + t.Run("discovery mode ABAC inactive returns all registry actions in results", func(t *testing.T) { + disableABAC(t) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + }) + require.Nil(t, appErr) + require.Len(t, resp.Decisions, 2) + require.Len(t, resp.Results, 2) + resultNames := make(map[string]bool, len(resp.Results)) + for _, r := range resp.Results { + resultNames[r.Action.Name] = true + } + require.True(t, resultNames[model.AccessControlPolicyActionUploadFileAttachment]) + require.True(t, resultNames[model.AccessControlPolicyActionDownloadFileAttachment]) + }) + + t.Run("discovery mode ABAC active permitted in results denied excluded", func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Action == model.AccessControlPolicyActionUploadFileAttachment + })).Return(model.AccessDecision{Decision: true}, (*model.AppError)(nil)) + mockACS.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Action == model.AccessControlPolicyActionDownloadFileAttachment + })).Return(model.AccessDecision{Decision: false}, (*model.AppError)(nil)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + }) + require.Nil(t, appErr) + require.Len(t, resp.Decisions, 2) + require.Len(t, resp.Results, 1) + require.Equal(t, model.AccessControlPolicyActionUploadFileAttachment, resp.Results[0].Action.Name) + }) + + t.Run("discovery mode wrong resource type returns empty candidates without error", func(t *testing.T) { + disableABAC(t) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: model.Resource{Type: model.AccessControlPolicyTypeTeam, ID: th.BasicTeam.Id}, + // No actions → discovery mode, but no registry entries for team type. + }) + require.Nil(t, appErr) + require.Empty(t, resp.Decisions) + require.Empty(t, resp.Results) + }) + + t.Run("discovery mode results order is deterministic", func(t *testing.T) { + disableABAC(t) + + var firstOrder []string + for i := range 5 { + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + }) + require.Nil(t, appErr) + names := make([]string, len(resp.Results)) + for j, r := range resp.Results { + names[j] = r.Action.Name + } + if i == 0 { + firstOrder = names + } else { + require.Equal(t, firstOrder, names, "Results order changed on iteration %d", i) + } + } + }) + + t.Run("results is empty slice not nil when all denied", func(t *testing.T) { + enableABAC(t) + mockACS := withMockACS(t) + + mockACS.On("AccessEvaluation", mock.Anything, mock.Anything). + Return(model.AccessDecision{Decision: false}, (*model.AppError)(nil)) + + resp, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + }) + require.Nil(t, appErr) + require.Empty(t, resp.Results) + + // Wire check: "results":[] not "results":null + wire, err := json.Marshal(resp) + require.NoError(t, err) + require.Contains(t, string(wire), `"results":[]`) + require.Contains(t, string(wire), `"decisions":{`) + }) + + t.Run("subject matching session user is accepted", func(t *testing.T) { + disableABAC(t) + + _, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Subject: &model.ActionSearchSubject{ID: session.UserId}, + }) + require.Nil(t, appErr) + }) + + t.Run("subject not matching session user returns 403", func(t *testing.T) { + _, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Subject: &model.ActionSearchSubject{ID: model.NewId()}, + }) + require.NotNil(t, appErr) + require.Equal(t, 403, appErr.StatusCode) + }) + + t.Run("subject invalid ID format returns 400 from IsValid", func(t *testing.T) { + _, appErr := th.App.SearchAllowedActionsForCurrentUser(rctx, model.ActionSearchRequest{ + Resource: channelResource, + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Subject: &model.ActionSearchSubject{ID: "not-a-valid-id"}, + }) + require.NotNil(t, appErr) + require.Equal(t, 400, appErr.StatusCode) + }) +} diff --git a/server/channels/app/access_control_test.go b/server/channels/app/access_control_test.go index 6eeab165b869..0606bca8da6c 100644 --- a/server/channels/app/access_control_test.go +++ b/server/channels/app/access_control_test.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" "testing" + "time" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/request" @@ -58,6 +59,13 @@ func storeMockWithMaskingOff(tb testing.TB) *TestHelper { return th } +// stubACPEtagInvalidation stubs the render-ETag cache invalidation calls the policy publish paths +// make through the store, so mock-store tests that mutate a policy don't panic on them. +func stubACPEtagInvalidation(m *storemocks.AccessControlPolicyStore) { + m.On("InvalidateEtagForChannel", mock.Anything).Return().Maybe() + m.On("ClearEtagCache").Return().Maybe() +} + func TestCreateOrUpdateAccessControlPolicy(t *testing.T) { th := SetupConfig(t, func(cfg *model.Config) { cfg.FeatureFlags.AttributeValueMasking = false @@ -164,6 +172,9 @@ func TestCreateOrUpdateAccessControlPolicy(t *testing.T) { mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockChannelStore := storemocks.ChannelStore{} mockStore.On("Channel").Return(&mockChannelStore) + mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) + mockStore.On("AccessControlPolicy").Return(&mockACPStore) // publishChannelPolicyEnforcedUpdate is expected to invalidate the // channel cache and reload the channel for the WS payload. mockChannelStore.On("InvalidateChannel", channelID).Once() @@ -211,6 +222,7 @@ func TestCreateOrUpdateAccessControlPolicy(t *testing.T) { // A parent save fans out to both its channel and team children; // with no children of either kind, neither search yields a broadcast. mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("SearchPolicies", thMock.Context, mock.MatchedBy(func(s model.AccessControlPolicySearch) bool { return s.Type == model.AccessControlPolicyTypeChannel && s.ParentID == parentID @@ -280,6 +292,7 @@ func TestDeleteAccessControlPolicy(t *testing.T) { // channel-type policies must NOT trigger a parent fan-out search. mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() mockAccessControl := &mocks.AccessControlServiceInterface{} @@ -317,6 +330,7 @@ func TestDeleteAccessControlPolicy(t *testing.T) { mockChannelStore.On("Get", childChannelID, true).Return(&model.Channel{Id: childChannelID, Type: model.ChannelTypePrivate}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) // channelPolicyIDsWithImport (called pre-delete) returns one child. mockACPStore.On("SearchPolicies", thMock.Context, mock.MatchedBy(func(s model.AccessControlPolicySearch) bool { @@ -424,6 +438,9 @@ func TestDeleteAccessControlPolicy(t *testing.T) { mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockChannelStore := storemocks.ChannelStore{} mockStore.On("Channel").Return(&mockChannelStore) + mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) + mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockChannelStore.On("InvalidateChannel", channelID).Once() mockChannelStore.On("Get", channelID, true).Return(&model.Channel{Id: channelID, Type: model.ChannelTypePrivate}, nil).Once() @@ -1325,6 +1342,7 @@ func TestUnassignPoliciesFromChannels(t *testing.T) { mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockAccessControlPolicyStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockAccessControlPolicyStore) mockStore.On("AccessControlPolicy").Return(&mockAccessControlPolicyStore) // Mock SearchPolicies to return the child policy as a child of the parent mockAccessControlPolicyStore.On("SearchPolicies", thMock.Context, model.AccessControlPolicySearch{ @@ -2202,6 +2220,43 @@ func TestHasPermissionToFileAction(t *testing.T) { result := th.App.HasPermissionToFileAction(th.Context, th.BasicUser.Id, th.BasicUser.Roles, th.BasicChannel.Id, model.AccessControlPolicyActionDownloadFileAttachment) assert.True(t, result) }) + + // Above are the allow short-circuits; below are the two paths that actually withhold access. + t.Run("should deny when the policy denies the action", func(t *testing.T) { + mockAccessControl := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockAccessControl + + th.ConfigStore.SetReadOnlyFF(false) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = new(true) + cfg.FeatureFlags.PermissionPolicies = true + }) + + mockAccessControl.On("AccessEvaluation", mock.Anything, mock.MatchedBy(func(req model.AccessRequest) bool { + return req.Resource.ID == th.BasicChannel.Id && + req.Action == model.AccessControlPolicyActionDownloadFileAttachment + })).Return(model.AccessDecision{Decision: false}, (*model.AppError)(nil)) + + result := th.App.HasPermissionToFileAction(th.Context, th.BasicUser.Id, th.BasicUser.Roles, th.BasicChannel.Id, model.AccessControlPolicyActionDownloadFileAttachment) + assert.False(t, result) + }) + + t.Run("should deny when the evaluation errors (fail-secure)", func(t *testing.T) { + mockAccessControl := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockAccessControl + + th.ConfigStore.SetReadOnlyFF(false) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = new(true) + cfg.FeatureFlags.PermissionPolicies = true + }) + + mockAccessControl.On("AccessEvaluation", mock.Anything, mock.Anything). + Return(model.AccessDecision{}, model.NewAppError("AccessEvaluation", "app.pdp.access_evaluation.app_error", nil, "", http.StatusInternalServerError)) + + result := th.App.HasPermissionToFileAction(th.Context, th.BasicUser.Id, th.BasicUser.Roles, th.BasicChannel.Id, model.AccessControlPolicyActionDownloadFileAttachment) + assert.False(t, result) + }) } func TestResolveSystemRole(t *testing.T) { @@ -4156,6 +4211,7 @@ func TestHydrateChannelPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) // We register the AccessControlPolicy() accessor in case any other // path touches it, but `GetActionsForPolicy` MUST NOT be called // when PolicyEnforced is false — that's the whole point of the @@ -4179,6 +4235,7 @@ func TestHydrateChannelPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) channelID := model.NewId() @@ -4197,6 +4254,7 @@ func TestHydrateChannelPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) channelID := model.NewId() @@ -4214,6 +4272,7 @@ func TestHydrateChannelPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) channelID := model.NewId() @@ -4231,6 +4290,7 @@ func TestHydrateChannelPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) channelID := model.NewId() @@ -4250,6 +4310,7 @@ func TestHydrateChannelsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() appErr := thMock.App.HydrateChannelsPolicyActions(thMock.Context, nil) @@ -4263,6 +4324,7 @@ func TestHydrateChannelsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() channels := []*model.Channel{ @@ -4281,6 +4343,7 @@ func TestHydrateChannelsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) enforced1 := model.NewId() @@ -4321,6 +4384,7 @@ func TestHydrateChannelsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) enforced := model.NewId() @@ -4343,6 +4407,7 @@ func TestHydrateChannelsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) channels := []*model.Channel{{Id: model.NewId(), PolicyEnforced: true}} @@ -4372,6 +4437,7 @@ func TestGetChannelHydratesPolicyActions(t *testing.T) { Return(&model.Channel{Id: channelID, Type: model.ChannelTypePrivate, PolicyEnforced: true}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicy", thMock.Context, channelID). Return(map[string]bool{model.AccessControlPolicyActionMembership: true}, nil).Once() @@ -4394,6 +4460,7 @@ func TestGetChannelHydratesPolicyActions(t *testing.T) { Return(&model.Channel{Id: channelID, Type: model.ChannelTypePrivate, PolicyEnforced: false}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() channel, appErr := thMock.App.GetChannel(thMock.Context, channelID) @@ -4429,6 +4496,7 @@ func TestGetChannelsForTeamForUserHydratesPolicyActions(t *testing.T) { }, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicies", thMock.Context, []string{permChannelID}). Return(map[string]map[string]bool{ @@ -4473,6 +4541,7 @@ func TestGetChannelsForTeamForUserHydratesPolicyActions(t *testing.T) { Return(model.ChannelList{{Id: channelID, TeamId: teamID, PolicyEnforced: true}}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicies", thMock.Context, []string{channelID}). Return(map[string]map[string]bool{ @@ -4501,6 +4570,7 @@ func TestGetChannelsForTeamForUserHydratesPolicyActions(t *testing.T) { Return(model.ChannelList{{Id: channelID, TeamId: teamID, PolicyEnforced: true}}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicies", thMock.Context, []string{channelID}). Return(nil, errors.New("boom")).Once() @@ -4536,6 +4606,7 @@ func TestSearchChannelsHydratePolicyActions(t *testing.T) { }, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicies", thMock.Context, []string{channelID}). Return(map[string]map[string]bool{ @@ -4578,6 +4649,7 @@ func TestSearchChannelsHydratePolicyActions(t *testing.T) { Return(model.ChannelList{{Id: channelID, TeamId: teamID, PolicyEnforced: true}}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicies", thMock.Context, []string{channelID}). Return(map[string]map[string]bool{ @@ -4606,6 +4678,7 @@ func TestSearchChannelsHydratePolicyActions(t *testing.T) { Return(model.ChannelList{{Id: channelID, TeamId: teamID, PolicyEnforced: true}}, nil).Once() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("GetActionsForPolicies", thMock.Context, []string{channelID}). Return(nil, errors.New("boom")).Once() @@ -4714,6 +4787,7 @@ func TestPublishChannelPolicyEnforcedUpdateHydratesBroadcastPayload(t *testing.T Return(&model.Channel{Id: channelID, Type: model.ChannelTypePrivate, PolicyEnforced: true}, nil).Twice() mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) // Permission-only policy: hydrator must return an action set WITHOUT // the membership key. This is the bug-fix invariant the broadcast @@ -5184,6 +5258,7 @@ func TestUpdateAccessControlPoliciesActive_MaskingGuard(t *testing.T) { mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("SetActiveStatusMultiple", thMock.Context, mock.Anything).Return([]*model.AccessControlPolicy{policy}, nil).Once() @@ -5217,6 +5292,7 @@ func TestUpdateAccessControlPoliciesActive_MaskingGuard(t *testing.T) { mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("SetActiveStatusMultiple", thMock.Context, mock.Anything).Return([]*model.AccessControlPolicy{policy}, nil).Once() @@ -5254,6 +5330,7 @@ func TestUpdateAccessControlPoliciesActive_BroadcastsWebsocketEvents(t *testing. mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("SetActiveStatusMultiple", thMock.Context, mock.Anything).Return([]*model.AccessControlPolicy{policy}, nil).Once() @@ -5285,6 +5362,7 @@ func TestUpdateAccessControlPoliciesActive_BroadcastsWebsocketEvents(t *testing. mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) mockACPStore.On("SetActiveStatusMultiple", thMock.Context, mock.Anything).Return([]*model.AccessControlPolicy{policy}, nil).Once() @@ -5656,3 +5734,100 @@ func TestGetAccessControlFieldsAutocompleteNativeAttributes(t *testing.T) { } }) } + +func TestInvalidateAttributeViewCache(t *testing.T) { + th := SetupWithStoreMock(t) + ch := th.App.Srv().Channels() + + th.App.invalidateAttributeViewCache() + require.True(t, ch.attributeViewRefreshLast.IsZero(), "a write should mark the view stale") + + // No rate limit: a deferred write is a lost one, because a client asks for a render decision + // once per change event and never re-asks. + ch.attributeViewRefreshLast = time.Now() + for range 100 { + th.App.invalidateAttributeViewCache() + } + require.True(t, ch.attributeViewRefreshLast.IsZero(), "later writes should still mark the view stale") + + // A write landing during an in-flight refresh cannot reset the timer, so it flags a follow-up. + ch.attributeViewRefreshLast = time.Now() + ch.attributeViewRefreshMut.Lock() + th.App.invalidateAttributeViewCache() + ch.attributeViewRefreshMut.Unlock() + require.False(t, ch.attributeViewRefreshLast.IsZero()) + require.True(t, ch.attributeViewNeedsRefresh.Load(), "a write during a refresh should flag a follow-up") +} + +func TestAttributeBasedAccessControlEnabled(t *testing.T) { + newConfig := func(flag bool, abac *bool) *model.Config { + cfg := &model.Config{} + cfg.SetDefaults() + cfg.FeatureFlags.PermissionPolicies = flag + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = abac + return cfg + } + + assert.False(t, attributeBasedAccessControlEnabled(newConfig(false, model.NewPointer(true))), "flag off") + assert.False(t, attributeBasedAccessControlEnabled(newConfig(true, model.NewPointer(false))), "ABAC off") + assert.False(t, attributeBasedAccessControlEnabled(newConfig(true, nil)), "ABAC unset") + assert.True(t, attributeBasedAccessControlEnabled(newConfig(true, model.NewPointer(true))), "both on") +} + +func TestClearABACRenderCachesOnFlip(t *testing.T) { + newConfig := func(abac bool) *model.Config { + cfg := &model.Config{} + cfg.SetDefaults() + cfg.FeatureFlags.PermissionPolicies = true + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(abac) + return cfg + } + + setup := func(t *testing.T) (*Channels, *storemocks.AccessControlPolicyStore, *storemocks.AttributesStore) { + th := SetupWithStoreMock(t) + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockACP := &storemocks.AccessControlPolicyStore{} + mockStore.On("AccessControlPolicy").Return(mockACP).Maybe() + + mockAttributes := &storemocks.AttributesStore{} + mockStore.On("Attributes").Return(mockAttributes).Maybe() + + return th.App.Srv().Channels(), mockACP, mockAttributes + } + + t.Run("clears both caches when ABAC is switched on", func(t *testing.T) { + ch, mockACP, mockAttributes := setup(t) + mockACP.On("ClearEtagCache").Once() + mockAttributes.On("ClearUserPropertyValuesEpochCache").Once() + + ch.clearABACRenderCachesOnFlip(newConfig(false), newConfig(true)) + + mockACP.AssertExpectations(t) + mockAttributes.AssertExpectations(t) + }) + + t.Run("clears both caches when ABAC is switched off", func(t *testing.T) { + ch, mockACP, mockAttributes := setup(t) + mockACP.On("ClearEtagCache").Once() + mockAttributes.On("ClearUserPropertyValuesEpochCache").Once() + + ch.clearABACRenderCachesOnFlip(newConfig(true), newConfig(false)) + + mockACP.AssertExpectations(t) + mockAttributes.AssertExpectations(t) + }) + + t.Run("does nothing for an unrelated config change", func(t *testing.T) { + ch, mockACP, mockAttributes := setup(t) + + prev := newConfig(true) + next := newConfig(true) + next.ServiceSettings.SiteURL = model.NewPointer("http://localhost:8065") + + ch.clearABACRenderCachesOnFlip(prev, next) + + mockACP.AssertNotCalled(t, "ClearEtagCache") + mockAttributes.AssertNotCalled(t, "ClearUserPropertyValuesEpochCache") + }) +} diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index d4a7ca80a203..30b754bd7b18 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -4590,6 +4590,13 @@ func (a *App) cleanupChannelAccessControlPolicy(rctx request.CTX, channel *model ) } } + + // Drop the channel's cached render-ETag epoch: its policy row is gone, so a stale epoch + // would otherwise linger until the cache TTL. Gated, unlike the delete above: nothing can be + // cached for this channel if ABAC was off, and every archive would cost a cluster message. + if a.attributeBasedAccessControlEnabled() { + a.Srv().Store().AccessControlPolicy().InvalidateEtagForChannel(channel.Id) + } } // recommendedPublicChannelsScanPageSize is the per-page size used while diff --git a/server/channels/app/channels.go b/server/channels/app/channels.go index 130d01d2557d..45e2a4e8d35c 100644 --- a/server/channels/app/channels.go +++ b/server/channels/app/channels.go @@ -84,8 +84,9 @@ type Channels struct { AccessControl einterfaces.AccessControlServiceInterface Intune einterfaces.IntuneInterface - attributeViewRefreshMut sync.Mutex - attributeViewRefreshLast time.Time + attributeViewRefreshMut sync.Mutex + attributeViewRefreshLast time.Time + attributeViewNeedsRefresh atomic.Bool // These are used to prevent concurrent upload requests // for a given upload session which could cause inconsistencies @@ -306,6 +307,8 @@ func (ch *Channels) Start() error { } }) + ch.AddConfigListener(ch.clearABACRenderCachesOnFlip) + // TODO: This should be moved to the platform service. if err := ch.srv.platform.EnsureAsymmetricSigningKey(); err != nil { return errors.Wrapf(err, "unable to ensure asymmetric signing key") diff --git a/server/channels/app/helper_test.go b/server/channels/app/helper_test.go index 611b806bdc02..7a1b5316aba0 100644 --- a/server/channels/app/helper_test.go +++ b/server/channels/app/helper_test.go @@ -266,8 +266,21 @@ func useCustomPushNotificationServer(cfg *model.Config) { } func SetupWithStoreMock(tb testing.TB) *TestHelper { + return SetupConfigWithStoreMock(tb, nil) +} + +// SetupConfigWithStoreMock applies updateConfig before Server.Start() wires the config listeners +// that read from the store. Set the starting config here rather than via a later +// th.App.UpdateConfig, which fires those listeners against the store mock. +func SetupConfigWithStoreMock(tb testing.TB, updateConfig func(*model.Config)) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), false, false, useCustomPushNotificationServer, nil, tb) + setupConfig := func(cfg *model.Config) { + useCustomPushNotificationServer(cfg) + if updateConfig != nil { + updateConfig(cfg) + } + } + th := setupTestHelper(mockStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), false, false, setupConfig, nil, tb) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 4ba5c63d0564..7fb5d56ac572 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -1417,18 +1417,62 @@ func (a *App) GetPosts(rctx request.CTX, channelID string, offset int, limit int return postList, nil } -func (a *App) GetPostsEtag(channelID string, collapsedThreads bool) string { - if a.AutoTranslation() == nil || !a.AutoTranslation().IsFeatureAvailable() { - return a.Srv().Store().Post().GetEtag(channelID, true, collapsedThreads, false) +// Cannot collide with a real epoch, which always carries a row count. For when no user is in +// scope and there is genuinely no attribute component to track. +const unknownABACEtagEpoch = "unknown" + +// degradedABACEtagEpoch stands in for an epoch whose lookup failed, and must miss the cache. +// Unique per call: a stable sentinel would let two failed lookups either side of a policy change +// produce identical ETags, and the client would 304 onto the old sanitization. +func degradedABACEtagEpoch() string { + return unknownABACEtagEpoch + "-" + strconv.FormatInt(time.Now().UnixNano(), 10) +} + +// AppendABACEtag folds the policy and user-attribute epochs into a base ETag, so a policy or +// attribute change misses the cache and SanitizePostListMetadataForUser runs instead of the +// request 304ing onto differently-sanitized content. No-op when ABAC is inactive. +// +// Pass "" for channelID when no channel is in scope; the policy epoch then covers only the +// system-scoped permission policies. +func (a *App) AppendABACEtag(base string, userID string, channelID string) string { + if !a.attributeBasedAccessControlEnabled() { + return base } - channelEnabled, err := a.AutoTranslation().IsChannelEnabled(channelID) - if err != nil || !channelEnabled { - return a.Srv().Store().Post().GetEtag(channelID, true, collapsedThreads, false) + rctx := request.EmptyContext(a.Log()) + + policyEpoch := degradedABACEtagEpoch() + if epoch, err := a.Srv().Store().AccessControlPolicy().GetEtagEpoch(rctx, channelID); err == nil { + policyEpoch = epoch + } else { + a.Log().Warn("ABAC ETag: failed to get access control policy epoch; policy component will be unknown", + mlog.Err(err)) } - // Channel has auto-translation enabled - include translation etag - return a.Srv().Store().Post().GetEtag(channelID, true, collapsedThreads, true) + cpaEpoch := unknownABACEtagEpoch + if userID != "" { + cpaEpoch = degradedABACEtagEpoch() + if epoch, err := a.Srv().Store().Attributes().GetUserPropertyValuesEpoch(rctx, userID); err == nil { + cpaEpoch = epoch + } else { + a.Log().Warn("ABAC ETag: failed to get user CPA epoch; attribute component will be unknown", + mlog.String("user_id", userID), + mlog.Err(err)) + } + } + + return fmt.Sprintf("%s.%s.%s", base, policyEpoch, cpaEpoch) +} + +func (a *App) GetPostsEtag(channelID string, userID string, collapsedThreads bool) string { + includeTranslations := false + if a.AutoTranslation() != nil && a.AutoTranslation().IsFeatureAvailable() { + if enabled, err := a.AutoTranslation().IsChannelEnabled(channelID); err == nil && enabled { + includeTranslations = true + } + } + base := a.Srv().Store().Post().GetEtag(channelID, true, collapsedThreads, includeTranslations) + return a.AppendABACEtag(base, userID, channelID) } func (a *App) GetPostsSince(rctx request.CTX, options model.GetPostsSinceOptions) (*model.PostList, *model.AppError) { diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index 54016d03fbe9..d3a43a532539 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -6765,3 +6765,73 @@ func TestGetPostsForView(t *testing.T) { assert.Empty(t, postList.Posts) }) } + +func TestAppendABACEtag(t *testing.T) { + const base = "16.2.3.abcdefghij.1700000000000" + + setup := func(t *testing.T, abacEnabled bool) (*TestHelper, *storemocks.AccessControlPolicyStore, *storemocks.AttributesStore) { + th := SetupConfigWithStoreMock(t, func(cfg *model.Config) { + cfg.FeatureFlags.PermissionPolicies = true + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(abacEnabled) + }) + mockStore := th.App.Srv().Store().(*storemocks.Store) + + mockACP := &storemocks.AccessControlPolicyStore{} + mockStore.On("AccessControlPolicy").Return(mockACP).Maybe() + + mockAttributes := &storemocks.AttributesStore{} + mockStore.On("Attributes").Return(mockAttributes).Maybe() + + return th, mockACP, mockAttributes + } + + t.Run("returns the base ETag untouched and reads no store when ABAC is off", func(t *testing.T) { + th, mockACP, mockAttributes := setup(t, false) + + assert.Equal(t, base, th.App.AppendABACEtag(base, model.NewId(), model.NewId())) + + mockACP.AssertNotCalled(t, "GetEtagEpoch", mock.Anything, mock.Anything) + mockAttributes.AssertNotCalled(t, "GetUserPropertyValuesEpoch", mock.Anything, mock.Anything) + }) + + t.Run("folds both epochs in when ABAC is on", func(t *testing.T) { + th, mockACP, mockAttributes := setup(t, true) + userID := model.NewId() + channelID := model.NewId() + + mockACP.On("GetEtagEpoch", mock.Anything, channelID).Return("111-2", nil).Once() + mockAttributes.On("GetUserPropertyValuesEpoch", mock.Anything, userID).Return("222-3", nil).Once() + + assert.Equal(t, base+".111-2.222-3", th.App.AppendABACEtag(base, userID, channelID)) + + mockACP.AssertExpectations(t) + mockAttributes.AssertExpectations(t) + }) + + t.Run("skips the attribute epoch when there is no user in scope", func(t *testing.T) { + th, mockACP, mockAttributes := setup(t, true) + channelID := model.NewId() + + mockACP.On("GetEtagEpoch", mock.Anything, channelID).Return("111-2", nil).Once() + + assert.Equal(t, base+".111-2."+unknownABACEtagEpoch, th.App.AppendABACEtag(base, "", channelID)) + + mockAttributes.AssertNotCalled(t, "GetUserPropertyValuesEpoch", mock.Anything, mock.Anything) + }) + + t.Run("a store failure yields an ETag that cannot match the healthy one", func(t *testing.T) { + th, mockACP, mockAttributes := setup(t, true) + userID := model.NewId() + channelID := model.NewId() + + mockACP.On("GetEtagEpoch", mock.Anything, channelID).Return("", errors.New("boom")).Twice() + mockAttributes.On("GetUserPropertyValuesEpoch", mock.Anything, userID).Return("222-3", nil).Twice() + + etag := th.App.AppendABACEtag(base, userID, channelID) + + assert.NotEqual(t, base, etag, "a failed epoch lookup must not collapse back onto the ungated ETag") + assert.Contains(t, etag, unknownABACEtagEpoch) + assert.NotEqual(t, etag, th.App.AppendABACEtag(base, userID, channelID), + "two failed lookups must not produce the same ETag, or a policy change between them would 304") + }) +} diff --git a/server/channels/app/property_field.go b/server/channels/app/property_field.go index 9d3fd2b93133..7abd44123cf7 100644 --- a/server/channels/app/property_field.go +++ b/server/channels/app/property_field.go @@ -399,6 +399,12 @@ func (a *App) UpdatePropertyFields(rctx request.CTX, groupID string, fields []*m } } + // Renaming or re-ranking an option rewrites what the matview materializes, and a type change + // drops dependent values, none of it touching the rows the per-user epoch is computed from. + if anyUserObjectType(updated) || anyUserObjectType(propagated) { + a.invalidateAllUserAttributeCaches() + } + // Broadcast websocket events for both requested and propagated fields for _, field := range updated { a.publishPropertyFieldEvent(rctx, model.WebsocketEventPropertyFieldUpdated, field, connectionID) @@ -421,6 +427,17 @@ func (a *App) UpdatePropertyFields(rctx request.CTX, groupID string, fields []*m return updated, clearedFieldIDs, nil } +// anyUserObjectType reports whether any field is one the AttributeView materializes, and that ABAC +// policies can therefore match on. +func anyUserObjectType(fields []*model.PropertyField) bool { + for _, f := range fields { + if f != nil && f.ObjectType == model.PropertyFieldObjectTypeUser { + return true + } + } + return false +} + // DeletePropertyField deletes a property field. func (a *App) DeletePropertyField(rctx request.CTX, groupID, id string, bypassProtectedCheck bool, connectionID string) *model.AppError { existing, err := a.Srv().propertyService.GetPropertyField(rctx, groupID, id) @@ -456,6 +473,12 @@ func (a *App) DeletePropertyField(rctx request.CTX, groupID, id string, bypassPr // compiled-policy cache entries that depend on this field are dropped // cluster-wide. Without this a deleted rank field's stale options would // linger in the per-node cache until restart. + // The matview filters out soft-deleted fields, so this attribute disappears from every subject + // while PropertyValues stays untouched and the per-user epoch cannot see it. + if existing.ObjectType == model.PropertyFieldObjectTypeUser { + a.invalidateAllUserAttributeCaches() + } + if acs := a.Srv().ch.AccessControl; acs != nil { acs.OnPropertyFieldOptionsChanged(rctx, existing.ID) } diff --git a/server/channels/app/property_value.go b/server/channels/app/property_value.go index 8892deb00589..9b9128f54940 100644 --- a/server/channels/app/property_value.go +++ b/server/channels/app/property_value.go @@ -37,6 +37,65 @@ func (a *App) resolveValueBroadcastParams(rctx request.CTX, objectType, targetID } } +// invalidateUserPropertyValuesEpochs marks the AttributeView stale and drops the cached CPA epoch +// for every distinct user the values target. It keys off each value's own TargetType/TargetID +// rather than a caller-supplied object type, to stay correct for plugin callers that pass none. +// +// Both halves must move together: the epoch alone forces a refetch that renders from a stale view +// and then caches that under the new ETag, and the view alone leaves the client 304ing on content +// sanitized under the old attributes. +func (a *App) invalidateUserPropertyValuesEpochs(values []*model.PropertyValue) { + seen := make(map[string]bool, len(values)) + for _, v := range values { + if v == nil || v.TargetType != model.PropertyFieldObjectTypeUser || seen[v.TargetID] { + continue + } + seen[v.TargetID] = true + } + if len(seen) == 0 { + return + } + + a.invalidateAttributeViewCache() + + // Only the epoch half is gated: nothing reads it while ABAC is off, and each invalidation + // costs a cluster message. A flip clears both caches wholesale, so nothing skipped here + // survives as a stale entry. + if !a.attributeBasedAccessControlEnabled() { + return + } + for targetID := range seen { + a.Srv().Store().Attributes().InvalidateUserPropertyValuesEpoch(targetID) + } +} + +// invalidateUserPropertyValuesEpoch is the single-target form, for delete paths that have a target +// ID rather than a set of values. Same split gate as above. +func (a *App) invalidateUserPropertyValuesEpoch(targetType, targetID string) { + if targetType != model.PropertyFieldObjectTypeUser { + return + } + + a.invalidateAttributeViewCache() + + if !a.attributeBasedAccessControlEnabled() { + return + } + a.Srv().Store().Attributes().InvalidateUserPropertyValuesEpoch(targetID) +} + +// invalidateAllUserAttributeCaches marks the AttributeView stale and drops every cached CPA epoch. +// For field mutations, which change what every subject resolves to without touching a single +// PropertyValues row, and so are invisible to an epoch computed from that table. +func (a *App) invalidateAllUserAttributeCaches() { + a.invalidateAttributeViewCache() + + if !a.attributeBasedAccessControlEnabled() { + return + } + a.Srv().Store().Attributes().ClearUserPropertyValuesEpochCache() +} + // CreatePropertyValue creates a new property value. func (a *App) CreatePropertyValue(rctx request.CTX, value *model.PropertyValue) (*model.PropertyValue, *model.AppError) { if value == nil { @@ -51,6 +110,8 @@ func (a *App) CreatePropertyValue(rctx request.CTX, value *model.PropertyValue) } return nil, model.NewAppError("CreatePropertyValue", "app.property_value.create.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + + a.invalidateUserPropertyValuesEpochs([]*model.PropertyValue{createdValue}) return createdValue, nil } @@ -70,6 +131,8 @@ func (a *App) CreatePropertyValues(rctx request.CTX, values []*model.PropertyVal } return nil, model.NewAppError("CreatePropertyValues", "app.property_value.create_many.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + + a.invalidateUserPropertyValuesEpochs(createdValues) return createdValues, nil } @@ -123,6 +186,8 @@ func (a *App) UpdatePropertyValue(rctx request.CTX, groupID string, value *model } return nil, model.NewAppError("UpdatePropertyValue", "app.property_value.update.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + + a.invalidateUserPropertyValuesEpochs([]*model.PropertyValue{updatedValue}) return updatedValue, nil } @@ -142,6 +207,8 @@ func (a *App) UpdatePropertyValues(rctx request.CTX, groupID string, values []*m } return nil, model.NewAppError("UpdatePropertyValues", "app.property_value.update_many.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + + a.invalidateUserPropertyValuesEpochs(updatedValues) return updatedValues, nil } @@ -159,6 +226,8 @@ func (a *App) UpsertPropertyValue(rctx request.CTX, value *model.PropertyValue) } return nil, model.NewAppError("UpsertPropertyValue", "app.property_value.upsert.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + + a.invalidateUserPropertyValuesEpochs([]*model.PropertyValue{upsertedValue}) return upsertedValue, nil } @@ -264,6 +333,8 @@ func (a *App) UpsertPropertyValues(rctx request.CTX, values []*model.PropertyVal return nil, model.NewAppError("UpsertPropertyValues", "app.property_value.upsert_many.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + a.invalidateUserPropertyValuesEpochs(result) + // Only publish websocket events for PSAv2 properties (those with an ObjectType) if objectType != "" { teamID, channelID, appErr := a.resolveValueBroadcastParams(rctx, objectType, targetID) @@ -300,6 +371,8 @@ func (a *App) DeletePropertyValue(rctx request.CTX, groupID, valueID string) *mo return model.NewAppError("DeletePropertyValue", "app.property_value.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + a.invalidateUserPropertyValuesEpoch(value.TargetType, value.TargetID) + teamID, channelID, appErr := a.resolveValueBroadcastParams(rctx, value.TargetType, value.TargetID) if appErr != nil { rctx.Logger().Warn("Failed to resolve broadcast params for property value deletion", mlog.Err(appErr)) @@ -335,6 +408,8 @@ func (a *App) DeletePropertyValuesForTarget(rctx request.CTX, groupID, targetTyp return model.NewAppError("DeletePropertyValuesForTarget", "app.property_value.delete_for_target.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + a.invalidateUserPropertyValuesEpoch(targetType, targetID) + teamID, channelID, appErr := a.resolveValueBroadcastParams(rctx, targetType, targetID) if appErr != nil { rctx.Logger().Warn("Failed to resolve broadcast params for property value deletion", mlog.Err(appErr)) @@ -358,6 +433,10 @@ func (a *App) DeletePropertyValuesForField(rctx request.CTX, groupID, fieldID st return model.NewAppError("DeletePropertyValuesForField", "app.property_value.delete_for_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + // A field delete can drop values for many users at once, so clear the whole epoch cache + // rather than trying to enumerate the affected users. + a.invalidateAllUserAttributeCaches() + message := model.NewWebSocketEvent(model.WebsocketEventPropertyValuesUpdated, "", "", "", nil, "") message.Add("field_id", fieldID) message.Add("values", "[]") diff --git a/server/channels/app/property_value_test.go b/server/channels/app/property_value_test.go index 2ed0ea7dbd94..9d0562de21a5 100644 --- a/server/channels/app/property_value_test.go +++ b/server/channels/app/property_value_test.go @@ -6,9 +6,12 @@ package app import ( "net/http" "testing" + "time" "github.com/mattermost/mattermost/server/public/model" + storemocks "github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -159,3 +162,91 @@ func TestUpsertPropertyValues_Invariants(t *testing.T) { } }) } + +func TestInvalidateUserPropertyValuesEpochs_GatedOnABAC(t *testing.T) { + userID := model.NewId() + + // Returns the attributes store mock so assertions can name the method rather than the + // Store.Attributes() accessor, which the config listener also calls. + setup := func(t *testing.T, flag bool, abac bool) (*TestHelper, *storemocks.AttributesStore) { + thMock := SetupConfigWithStoreMock(t, func(cfg *model.Config) { + cfg.FeatureFlags.PermissionPolicies = flag + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(abac) + }) + + mockAttributes := &storemocks.AttributesStore{} + thMock.App.Srv().Store().(*storemocks.Store).On("Attributes").Return(mockAttributes).Maybe() + + return thMock, mockAttributes + } + + values := []*model.PropertyValue{ + {TargetType: model.PropertyFieldObjectTypeUser, TargetID: userID}, + {TargetType: model.PropertyFieldObjectTypeUser, TargetID: userID}, // duplicate, one invalidation + {TargetType: model.PropertyFieldObjectTypePost, TargetID: model.NewId()}, + } + + t.Run("no invalidation when the feature flag is off", func(t *testing.T) { + thMock, mockAttributes := setup(t, false, true) + + thMock.App.invalidateUserPropertyValuesEpochs(values) + thMock.App.invalidateUserPropertyValuesEpoch(model.PropertyFieldObjectTypeUser, userID) + + mockAttributes.AssertNotCalled(t, "InvalidateUserPropertyValuesEpoch", mock.Anything) + }) + + t.Run("no invalidation when ABAC is off", func(t *testing.T) { + thMock, mockAttributes := setup(t, true, false) + + thMock.App.invalidateUserPropertyValuesEpochs(values) + + mockAttributes.AssertNotCalled(t, "InvalidateUserPropertyValuesEpoch", mock.Anything) + }) + + t.Run("one invalidation per distinct user target when active", func(t *testing.T) { + thMock, mockAttributes := setup(t, true, true) + mockAttributes.On("InvalidateUserPropertyValuesEpoch", userID).Once() + + thMock.App.invalidateUserPropertyValuesEpochs(values) + + mockAttributes.AssertExpectations(t) + }) + + t.Run("single-target form skips non-user targets", func(t *testing.T) { + thMock, mockAttributes := setup(t, true, true) + + thMock.App.invalidateUserPropertyValuesEpoch(model.PropertyFieldObjectTypePost, model.NewId()) + + mockAttributes.AssertNotCalled(t, "InvalidateUserPropertyValuesEpoch", mock.Anything) + }) + + // A write made while ABAC was off must not leave the view stale for a switch back on to read, + // and the marker is free, so only the epoch half is gated. + t.Run("the view is marked stale even when ABAC is off", func(t *testing.T) { + thMock, _ := setup(t, true, false) + ch := thMock.App.Srv().Channels() + + ch.attributeViewRefreshLast = time.Now() + thMock.App.invalidateUserPropertyValuesEpochs(values) + require.True(t, ch.attributeViewRefreshLast.IsZero(), "a user-targeted write should mark the view stale") + + ch.attributeViewRefreshLast = time.Now() + thMock.App.invalidateUserPropertyValuesEpoch(model.PropertyFieldObjectTypeUser, userID) + require.True(t, ch.attributeViewRefreshLast.IsZero(), "the single-target form should too") + }) + + t.Run("no user target means no invalidation of either kind", func(t *testing.T) { + thMock, mockAttributes := setup(t, true, true) + ch := thMock.App.Srv().Channels() + now := time.Now() + ch.attributeViewRefreshLast = now + + thMock.App.invalidateUserPropertyValuesEpochs([]*model.PropertyValue{ + {TargetType: model.PropertyFieldObjectTypePost, TargetID: model.NewId()}, + nil, + }) + + require.Equal(t, now, ch.attributeViewRefreshLast) + mockAttributes.AssertNotCalled(t, "InvalidateUserPropertyValuesEpoch", mock.Anything) + }) +} diff --git a/server/channels/app/team_membership_access_control_test.go b/server/channels/app/team_membership_access_control_test.go index 8a6a609864fc..a007dcea182c 100644 --- a/server/channels/app/team_membership_access_control_test.go +++ b/server/channels/app/team_membership_access_control_test.go @@ -22,6 +22,7 @@ func TestHydrateTeamPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() team := &model.Team{Id: model.NewId(), PolicyEnforced: false} @@ -41,6 +42,7 @@ func TestHydrateTeamPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) teamID := model.NewId() @@ -58,6 +60,7 @@ func TestHydrateTeamPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) teamID := model.NewId() @@ -74,6 +77,7 @@ func TestHydrateTeamPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) teamID := model.NewId() @@ -91,6 +95,7 @@ func TestHydrateTeamPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) teamID := model.NewId() @@ -110,6 +115,7 @@ func TestHydrateTeamsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() appErr := thMock.App.HydrateTeamsPolicyActions(thMock.Context, nil) @@ -123,6 +129,7 @@ func TestHydrateTeamsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore).Maybe() teams := []*model.Team{ @@ -141,6 +148,7 @@ func TestHydrateTeamsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) enforced1 := model.NewId() @@ -180,6 +188,7 @@ func TestHydrateTeamsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) enforced := model.NewId() @@ -197,6 +206,7 @@ func TestHydrateTeamsPolicyActions(t *testing.T) { thMock := SetupWithStoreMock(t) mockStore := thMock.App.Srv().Store().(*storemocks.Store) mockACPStore := storemocks.AccessControlPolicyStore{} + stubACPEtagInvalidation(&mockACPStore) mockStore.On("AccessControlPolicy").Return(&mockACPStore) teams := []*model.Team{{Id: model.NewId(), PolicyEnforced: true}} diff --git a/server/channels/store/localcachelayer/access_control_policy_layer.go b/server/channels/store/localcachelayer/access_control_policy_layer.go new file mode 100644 index 000000000000..8ca1a3adf38d --- /dev/null +++ b/server/channels/store/localcachelayer/access_control_policy_layer.go @@ -0,0 +1,60 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package localcachelayer + +import ( + "bytes" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/channels/store" +) + +type LocalCacheAccessControlPolicyStore struct { + store.AccessControlPolicyStore + rootStore *LocalCacheStore +} + +func (s *LocalCacheAccessControlPolicyStore) handleClusterInvalidateAccessControlPolicyEtag(msg *model.ClusterMessage) { + if bytes.Equal(msg.Data, clearCacheMessageData) { + if err := s.rootStore.accessControlPolicyEtagCache.Purge(); err != nil { + s.rootStore.logger.Warn("failed to purge access control policy etag cache", mlog.Err(err)) + } + } else if err := s.rootStore.accessControlPolicyEtagCache.Remove(string(msg.Data)); err != nil { + s.rootStore.logger.Warn("failed to remove access control policy etag cache entry", mlog.Err(err)) + } +} + +// Keyed by channel ID, with the empty string as the system-scoped-only bucket. Every channel's +// epoch folds in the permission policies, so a permission-policy change has to clear the whole +// cache; a single channel's change only invalidates its own key. +func (s LocalCacheAccessControlPolicyStore) GetEtagEpoch(rctx request.CTX, channelID string) (string, error) { + var epoch string + if err := s.rootStore.doStandardReadCache(s.rootStore.accessControlPolicyEtagCache, channelID, &epoch); err == nil { + return epoch, nil + } + + epoch, err := s.AccessControlPolicyStore.GetEtagEpoch(rctx, channelID) + if err != nil { + return "", err + } + + s.rootStore.doStandardAddToCache(s.rootStore.accessControlPolicyEtagCache, channelID, epoch) + return epoch, nil +} + +func (s LocalCacheAccessControlPolicyStore) InvalidateEtagForChannel(channelID string) { + s.rootStore.doInvalidateCacheCluster(s.rootStore.accessControlPolicyEtagCache, channelID, nil) + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter(s.rootStore.accessControlPolicyEtagCache.Name()) + } +} + +func (s LocalCacheAccessControlPolicyStore) ClearEtagCache() { + s.rootStore.doClearCacheCluster(s.rootStore.accessControlPolicyEtagCache) + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter(s.rootStore.accessControlPolicyEtagCache.Name()) + } +} diff --git a/server/channels/store/localcachelayer/access_control_policy_layer_test.go b/server/channels/store/localcachelayer/access_control_policy_layer_test.go new file mode 100644 index 000000000000..12d73cc6e599 --- /dev/null +++ b/server/channels/store/localcachelayer/access_control_policy_layer_test.go @@ -0,0 +1,69 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package localcachelayer + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks" +) + +func TestAccessControlPolicyStoreCache(t *testing.T) { + channelID := "channel-id" + logger := mlog.CreateConsoleTestLogger(t) + rctx := request.TestContext(t) + + t.Run("GetEtagEpoch cached on second call", func(t *testing.T) { + mockStore := getMockStore(t) + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, getMockCacheProvider(), logger) + require.NoError(t, err) + + epoch, err := cachedStore.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + assert.Equal(t, "100-1", epoch) + mockStore.AccessControlPolicy().(*mocks.AccessControlPolicyStore).AssertNumberOfCalls(t, "GetEtagEpoch", 1) + + epoch, err = cachedStore.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + assert.Equal(t, "100-1", epoch) + mockStore.AccessControlPolicy().(*mocks.AccessControlPolicyStore).AssertNumberOfCalls(t, "GetEtagEpoch", 1) + }) + + t.Run("InvalidateEtagForChannel forces a re-query", func(t *testing.T) { + mockStore := getMockStore(t) + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, getMockCacheProvider(), logger) + require.NoError(t, err) + + _, err = cachedStore.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + mockStore.AccessControlPolicy().(*mocks.AccessControlPolicyStore).AssertNumberOfCalls(t, "GetEtagEpoch", 1) + + cachedStore.AccessControlPolicy().InvalidateEtagForChannel(channelID) + + _, err = cachedStore.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + mockStore.AccessControlPolicy().(*mocks.AccessControlPolicyStore).AssertNumberOfCalls(t, "GetEtagEpoch", 2) + }) + + t.Run("ClearEtagCache forces a re-query", func(t *testing.T) { + mockStore := getMockStore(t) + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, getMockCacheProvider(), logger) + require.NoError(t, err) + + _, err = cachedStore.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + mockStore.AccessControlPolicy().(*mocks.AccessControlPolicyStore).AssertNumberOfCalls(t, "GetEtagEpoch", 1) + + cachedStore.AccessControlPolicy().ClearEtagCache() + + _, err = cachedStore.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + mockStore.AccessControlPolicy().(*mocks.AccessControlPolicyStore).AssertNumberOfCalls(t, "GetEtagEpoch", 2) + }) +} diff --git a/server/channels/store/localcachelayer/attributes_layer.go b/server/channels/store/localcachelayer/attributes_layer.go new file mode 100644 index 000000000000..e842420e6511 --- /dev/null +++ b/server/channels/store/localcachelayer/attributes_layer.go @@ -0,0 +1,59 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package localcachelayer + +import ( + "bytes" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/channels/store" +) + +type LocalCacheAttributesStore struct { + store.AttributesStore + rootStore *LocalCacheStore +} + +func (s *LocalCacheAttributesStore) handleClusterInvalidateUserPropertyValuesEpoch(msg *model.ClusterMessage) { + if bytes.Equal(msg.Data, clearCacheMessageData) { + if err := s.rootStore.userPropertyValuesEpochCache.Purge(); err != nil { + s.rootStore.logger.Warn("failed to purge user property values epoch cache", mlog.Err(err)) + } + } else if err := s.rootStore.userPropertyValuesEpochCache.Remove(string(msg.Data)); err != nil { + s.rootStore.logger.Warn("failed to remove user property values epoch cache entry", mlog.Err(err)) + } +} + +// Keyed by user ID, so the ABAC-aware post-list ETag doesn't hit PropertyValues on every post GET. +// The App layer invalidates the key on property-value writes. +func (s LocalCacheAttributesStore) GetUserPropertyValuesEpoch(rctx request.CTX, userID string) (string, error) { + var epoch string + if err := s.rootStore.doStandardReadCache(s.rootStore.userPropertyValuesEpochCache, userID, &epoch); err == nil { + return epoch, nil + } + + epoch, err := s.AttributesStore.GetUserPropertyValuesEpoch(rctx, userID) + if err != nil { + return "", err + } + + s.rootStore.doStandardAddToCache(s.rootStore.userPropertyValuesEpochCache, userID, epoch) + return epoch, nil +} + +func (s LocalCacheAttributesStore) InvalidateUserPropertyValuesEpoch(userID string) { + s.rootStore.doInvalidateCacheCluster(s.rootStore.userPropertyValuesEpochCache, userID, nil) + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter(s.rootStore.userPropertyValuesEpochCache.Name()) + } +} + +func (s LocalCacheAttributesStore) ClearUserPropertyValuesEpochCache() { + s.rootStore.doClearCacheCluster(s.rootStore.userPropertyValuesEpochCache) + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter(s.rootStore.userPropertyValuesEpochCache.Name()) + } +} diff --git a/server/channels/store/localcachelayer/attributes_layer_test.go b/server/channels/store/localcachelayer/attributes_layer_test.go new file mode 100644 index 000000000000..2377ff6734fe --- /dev/null +++ b/server/channels/store/localcachelayer/attributes_layer_test.go @@ -0,0 +1,69 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package localcachelayer + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks" +) + +func TestAttributesStoreCache(t *testing.T) { + userID := "user-id" + logger := mlog.CreateConsoleTestLogger(t) + rctx := request.TestContext(t) + + t.Run("GetUserPropertyValuesEpoch cached on second call", func(t *testing.T) { + mockStore := getMockStore(t) + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, getMockCacheProvider(), logger) + require.NoError(t, err) + + epoch, err := cachedStore.Attributes().GetUserPropertyValuesEpoch(rctx, userID) + require.NoError(t, err) + assert.Equal(t, "200-1", epoch) + mockStore.Attributes().(*mocks.AttributesStore).AssertNumberOfCalls(t, "GetUserPropertyValuesEpoch", 1) + + epoch, err = cachedStore.Attributes().GetUserPropertyValuesEpoch(rctx, userID) + require.NoError(t, err) + assert.Equal(t, "200-1", epoch) + mockStore.Attributes().(*mocks.AttributesStore).AssertNumberOfCalls(t, "GetUserPropertyValuesEpoch", 1) + }) + + t.Run("InvalidateUserPropertyValuesEpoch forces a re-query", func(t *testing.T) { + mockStore := getMockStore(t) + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, getMockCacheProvider(), logger) + require.NoError(t, err) + + _, err = cachedStore.Attributes().GetUserPropertyValuesEpoch(rctx, userID) + require.NoError(t, err) + mockStore.Attributes().(*mocks.AttributesStore).AssertNumberOfCalls(t, "GetUserPropertyValuesEpoch", 1) + + cachedStore.Attributes().InvalidateUserPropertyValuesEpoch(userID) + + _, err = cachedStore.Attributes().GetUserPropertyValuesEpoch(rctx, userID) + require.NoError(t, err) + mockStore.Attributes().(*mocks.AttributesStore).AssertNumberOfCalls(t, "GetUserPropertyValuesEpoch", 2) + }) + + t.Run("ClearUserPropertyValuesEpochCache forces a re-query", func(t *testing.T) { + mockStore := getMockStore(t) + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, getMockCacheProvider(), logger) + require.NoError(t, err) + + _, err = cachedStore.Attributes().GetUserPropertyValuesEpoch(rctx, userID) + require.NoError(t, err) + mockStore.Attributes().(*mocks.AttributesStore).AssertNumberOfCalls(t, "GetUserPropertyValuesEpoch", 1) + + cachedStore.Attributes().ClearUserPropertyValuesEpochCache() + + _, err = cachedStore.Attributes().GetUserPropertyValuesEpoch(rctx, userID) + require.NoError(t, err) + mockStore.Attributes().(*mocks.AttributesStore).AssertNumberOfCalls(t, "GetUserPropertyValuesEpoch", 2) + }) +} diff --git a/server/channels/store/localcachelayer/layer.go b/server/channels/store/localcachelayer/layer.go index 7ed30dc94b01..b92cc17c6a2b 100644 --- a/server/channels/store/localcachelayer/layer.go +++ b/server/channels/store/localcachelayer/layer.go @@ -90,6 +90,12 @@ const ( PropertyFieldCacheSize = 100 PropertyFieldCacheSec = 30 * 60 + + AccessControlPolicyEtagCacheSize = 25000 // Per-channel render-ETag epochs + AccessControlPolicyEtagCacheSec = 15 * 60 + + UserPropertyValuesEpochCacheSize = 25000 // Per-user CPA epochs + UserPropertyValuesEpochCacheSec = 15 * 60 ) var clearCacheMessageData = []byte("") @@ -166,6 +172,12 @@ type LocalCacheStore struct { propertyField LocalCachePropertyFieldStore propertyFieldCache cache.Cache + + accessControlPolicy LocalCacheAccessControlPolicyStore + accessControlPolicyEtagCache cache.Cache + + attributes LocalCacheAttributesStore + userPropertyValuesEpochCache cache.Cache } func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface, cacheProvider cache.Provider, logger mlog.LoggerIFace) (localCacheStore LocalCacheStore, err error) { @@ -491,6 +503,28 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf } localCacheStore.propertyField = LocalCachePropertyFieldStore{PropertyFieldStore: baseStore.PropertyField(), rootStore: &localCacheStore} + // Access Control Policy render-ETag epochs + if localCacheStore.accessControlPolicyEtagCache, err = cacheProvider.NewCache(&cache.CacheOptions{ + Size: AccessControlPolicyEtagCacheSize, + Name: "AccessControlPolicyEtag", + DefaultExpiry: AccessControlPolicyEtagCacheSec * time.Second, + InvalidateClusterEvent: model.ClusterEventInvalidateCacheForAccessControlPolicyEtag, + }); err != nil { + return + } + localCacheStore.accessControlPolicy = LocalCacheAccessControlPolicyStore{AccessControlPolicyStore: baseStore.AccessControlPolicy(), rootStore: &localCacheStore} + + // User property-values (CPA) epochs + if localCacheStore.userPropertyValuesEpochCache, err = cacheProvider.NewCache(&cache.CacheOptions{ + Size: UserPropertyValuesEpochCacheSize, + Name: "UserPropertyValuesEpoch", + DefaultExpiry: UserPropertyValuesEpochCacheSec * time.Second, + InvalidateClusterEvent: model.ClusterEventInvalidateCacheForUserPropertyValuesEpoch, + }); err != nil { + return + } + localCacheStore.attributes = LocalCacheAttributesStore{AttributesStore: baseStore.Attributes(), rootStore: &localCacheStore} + if cluster != nil { cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForReactions, localCacheStore.reaction.handleClusterInvalidateReaction) cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForRoles, localCacheStore.role.handleClusterInvalidateRole) @@ -522,6 +556,8 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForTemporaryPosts, localCacheStore.temporaryPost.handleClusterInvalidateTemporaryPosts) cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForSessionAttributes, localCacheStore.sessionAttribute.handleClusterInvalidateSessionAttributes) cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForPropertyFields, localCacheStore.propertyField.handleClusterInvalidatePropertyField) + cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForAccessControlPolicyEtag, localCacheStore.accessControlPolicy.handleClusterInvalidateAccessControlPolicyEtag) + cluster.RegisterClusterMessageHandler(model.ClusterEventInvalidateCacheForUserPropertyValuesEpoch, localCacheStore.attributes.handleClusterInvalidateUserPropertyValuesEpoch) } return } @@ -594,6 +630,14 @@ func (s LocalCacheStore) PropertyField() store.PropertyFieldStore { return s.propertyField } +func (s LocalCacheStore) AccessControlPolicy() store.AccessControlPolicyStore { + return s.accessControlPolicy +} + +func (s LocalCacheStore) Attributes() store.AttributesStore { + return s.attributes +} + func (s LocalCacheStore) DropAllTables() { s.Invalidate() s.Store.DropAllTables() @@ -741,6 +785,8 @@ func (s *LocalCacheStore) Invalidate() { s.doClearCacheCluster(s.temporaryPostCache) s.doClearCacheCluster(s.sessionAttributeCache) s.doClearCacheCluster(s.propertyFieldCache) + s.doClearCacheCluster(s.accessControlPolicyEtagCache) + s.doClearCacheCluster(s.userPropertyValuesEpochCache) } // allocateCacheTargets is used to fill target value types diff --git a/server/channels/store/localcachelayer/main_test.go b/server/channels/store/localcachelayer/main_test.go index 0f3fb1417f9f..c9b26fc3b8a7 100644 --- a/server/channels/store/localcachelayer/main_test.go +++ b/server/channels/store/localcachelayer/main_test.go @@ -227,6 +227,14 @@ func getMockStore(t *testing.T) *mocks.Store { mockPropertyFieldStore.On("Delete", "group-id", "field-id").Return(nil) mockStore.On("PropertyField").Return(&mockPropertyFieldStore) + mockAccessControlPolicyStore := mocks.AccessControlPolicyStore{} + mockAccessControlPolicyStore.On("GetEtagEpoch", mock.Anything, "channel-id").Return("100-1", nil) + mockStore.On("AccessControlPolicy").Return(&mockAccessControlPolicyStore) + + mockAttributesStore := mocks.AttributesStore{} + mockAttributesStore.On("GetUserPropertyValuesEpoch", mock.Anything, "user-id").Return("200-1", nil) + mockStore.On("Attributes").Return(&mockAttributesStore) + mockReadReceiptStore := &mocks.ReadReceiptStore{} mockStore.On("ReadReceipt").Return(mockReadReceiptStore) diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 718671354ce7..5d019a3e522a 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -642,6 +642,12 @@ func isRepeatableError(err error) bool { return false } +func (s *RetryLayerAccessControlPolicyStore) ClearEtagCache() { + + s.AccessControlPolicyStore.ClearEtagCache() + +} + func (s *RetryLayerAccessControlPolicyStore) Delete(rctx request.CTX, id string) error { tries := 0 @@ -726,6 +732,27 @@ func (s *RetryLayerAccessControlPolicyStore) GetActionsForPolicy(rctx request.CT } +func (s *RetryLayerAccessControlPolicyStore) GetEtagEpoch(rctx request.CTX, channelID string) (string, error) { + + tries := 0 + for { + result, err := s.AccessControlPolicyStore.GetEtagEpoch(rctx, channelID) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerAccessControlPolicyStore) GetPoliciesByFieldID(rctx request.CTX, fieldID string) ([]*model.AccessControlPolicy, error) { tries := 0 @@ -747,6 +774,12 @@ func (s *RetryLayerAccessControlPolicyStore) GetPoliciesByFieldID(rctx request.C } +func (s *RetryLayerAccessControlPolicyStore) InvalidateEtagForChannel(channelID string) { + + s.AccessControlPolicyStore.InvalidateEtagForChannel(channelID) + +} + func (s *RetryLayerAccessControlPolicyStore) Save(rctx request.CTX, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, error) { tries := 0 @@ -831,6 +864,12 @@ func (s *RetryLayerAccessControlPolicyStore) SetActiveStatusMultiple(rctx reques } +func (s *RetryLayerAttributesStore) ClearUserPropertyValuesEpochCache() { + + s.AttributesStore.ClearUserPropertyValuesEpochCache() + +} + func (s *RetryLayerAttributesStore) GetChannelMembersToRemove(rctx request.CTX, channelID string, opts model.SubjectSearchOptions) ([]*model.ChannelMember, error) { tries := 0 @@ -894,6 +933,33 @@ func (s *RetryLayerAttributesStore) GetTeamMembersToRemove(rctx request.CTX, tea } +func (s *RetryLayerAttributesStore) GetUserPropertyValuesEpoch(rctx request.CTX, userID string) (string, error) { + + tries := 0 + for { + result, err := s.AttributesStore.GetUserPropertyValuesEpoch(rctx, userID) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + +func (s *RetryLayerAttributesStore) InvalidateUserPropertyValuesEpoch(userID string) { + + s.AttributesStore.InvalidateUserPropertyValuesEpoch(userID) + +} + func (s *RetryLayerAttributesStore) RefreshAttributes() error { tries := 0 diff --git a/server/channels/store/sqlstore/access_control_policy_store.go b/server/channels/store/sqlstore/access_control_policy_store.go index 625350d2a6b3..62f92e79fa6b 100644 --- a/server/channels/store/sqlstore/access_control_policy_store.go +++ b/server/channels/store/sqlstore/access_control_policy_store.go @@ -941,3 +941,40 @@ func (s *SqlAccessControlPolicyStore) GetPoliciesByFieldID(_ request.CTX, fieldI return policies, nil } + +// GetEtagEpoch covers the system-scoped permission policies plus the given channel's own policy +// row (a channel policy's ID equals the channel ID); an empty channelID covers only the former. +// +// MAX(CreateAt), because the table has no UpdateAt and saves are delete-and-reinsert. The count is +// folded in to catch deletes: dropping a policy that isn't the newest leaves the max untouched, so +// the ETag would still match and clients would keep a differently-sanitized cached copy. +// +// Matching the channel by primary key instead of scanning every policy's rules JSON costs a benign +// false positive: a membership-only change to this channel's policy also advances the epoch. +func (s *SqlAccessControlPolicyStore) GetEtagEpoch(rctx request.CTX, channelID string) (string, error) { + query, args, err := s.getQueryBuilder(). + Select("COALESCE(MAX(CreateAt), 0) AS MaxCreateAt", "COUNT(*) AS Total"). + From("AccessControlPolicies"). + Where(sq.Or{ + sq.Eq{"Type": model.AccessControlPolicyTypePermission}, + sq.Eq{"Id": channelID}, + }). + ToSql() + if err != nil { + return "", errors.Wrap(err, "GetEtagEpoch: failed to build query") + } + + var epoch struct { + MaxCreateAt int64 + Total int64 + } + // Master, not replica — see the note on SqlAttributesStore.GetUserPropertyValuesEpoch. + if err := s.GetMaster().Get(&epoch, query, args...); err != nil { + return "", errors.Wrap(err, "GetEtagEpoch: query failed") + } + return fmt.Sprintf("%d-%d", epoch.MaxCreateAt, epoch.Total), nil +} + +// No-op at the SQL layer; the render-ETag epoch is cached in and invalidated by the local cache layer. +func (s *SqlAccessControlPolicyStore) InvalidateEtagForChannel(channelID string) {} +func (s *SqlAccessControlPolicyStore) ClearEtagCache() {} diff --git a/server/channels/store/sqlstore/attributes_store.go b/server/channels/store/sqlstore/attributes_store.go index b4e2e1611964..887239fbe095 100644 --- a/server/channels/store/sqlstore/attributes_store.go +++ b/server/channels/store/sqlstore/attributes_store.go @@ -279,6 +279,36 @@ func (s *SqlAttributesStore) GetChannelMembersToRemove(rctx request.CTX, channel return members, nil } +// Count folded in alongside the max to catch soft deletes: dropping a value that isn't the most +// recently updated leaves MAX(UpdateAt) unchanged, and the stale ETag would keep matching. +func (s *SqlAttributesStore) GetUserPropertyValuesEpoch(rctx request.CTX, userID string) (string, error) { + query, args, err := s.getQueryBuilder(). + Select("COALESCE(MAX(UpdateAt), 0) AS MaxUpdateAt", "COUNT(*) AS Total"). + From("PropertyValues"). + Where(sq.Eq{"TargetID": userID}). + Where("DeleteAt = 0"). + ToSql() + if err != nil { + return "", errors.Wrap(err, "GetUserPropertyValuesEpoch: failed to build query") + } + + var epoch struct { + MaxUpdateAt int64 + Total int64 + } + // Master, not replica: the caller invalidates this key right after the write commits, so a + // lagging replica would re-cache the pre-write epoch and the client would keep 304ing onto + // content sanitized under the old attributes until the entry expires. + if err := s.GetMaster().Get(&epoch, query, args...); err != nil { + return "", errors.Wrap(err, "GetUserPropertyValuesEpoch: query failed") + } + return fmt.Sprintf("%d-%d", epoch.MaxUpdateAt, epoch.Total), nil +} + +// No-op at the SQL layer; the per-user epoch is cached in and invalidated by the local cache layer. +func (s *SqlAttributesStore) InvalidateUserPropertyValuesEpoch(userID string) {} +func (s *SqlAttributesStore) ClearUserPropertyValuesEpochCache() {} + func (s *SqlAttributesStore) GetTeamMembersToRemove(rctx request.CTX, teamID string, opts model.SubjectSearchOptions) ([]*model.TeamMember, error) { query := s.getQueryBuilder(). Select(qualify("TeamMembers", teamMemberSliceColumns())...).From("TeamMembers"). diff --git a/server/channels/store/store.go b/server/channels/store/store.go index d1110d112670..17f22dfc935b 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1239,6 +1239,20 @@ type AccessControlPolicyStore interface { // channel-list reads to avoid an N+1 against AccessControlPolicies. // Empty input returns an empty map and fires no SQL. GetActionsForPolicies(rctx request.CTX, policyIDs []string) (map[string]map[string]bool, error) + + // GetEtagEpoch returns an opaque epoch over the system-scoped permission policies plus the + // given channel's own policy row; an empty channelID covers only the former. Deletion- + // sensitive, so it moves even when the newest policy is not the one that changed. + GetEtagEpoch(rctx request.CTX, channelID string) (string, error) + + // InvalidateEtagForChannel drops the cached render-ETag epoch for a single channel's policy. + // Call after that channel's policy changes. No-op outside the local cache layer. + InvalidateEtagForChannel(channelID string) + + // ClearEtagCache drops all cached render-ETag epochs. Call after a system-scoped permission + // policy changes, since every channel's epoch aggregates the permission set. No-op outside + // the local cache layer. + ClearEtagCache() } type AttributesStore interface { @@ -1246,7 +1260,19 @@ type AttributesStore interface { GetSubject(rctx request.CTX, ID, groupID, objectType string) (*model.Subject, error) SearchUsers(rctx request.CTX, opts model.SubjectSearchOptions) ([]*model.User, int64, error) GetChannelMembersToRemove(rctx request.CTX, channelID string, opts model.SubjectSearchOptions) ([]*model.ChannelMember, error) + // GetUserPropertyValuesEpoch returns the per-user epoch for ABAC-aware post-list ETags. + // Deletion-sensitive, so soft-deleting any value moves it. + GetUserPropertyValuesEpoch(rctx request.CTX, userID string) (string, error) GetTeamMembersToRemove(rctx request.CTX, teamID string, opts model.SubjectSearchOptions) ([]*model.TeamMember, error) + + // InvalidateUserPropertyValuesEpoch drops the cached property-values epoch for a single user. + // Call after that user's property values change. No-op outside the local cache layer. + InvalidateUserPropertyValuesEpoch(userID string) + + // ClearUserPropertyValuesEpochCache drops all cached property-values epochs. Call after a + // change that can affect many users at once (e.g. deleting a field). No-op outside the local + // cache layer. + ClearUserPropertyValuesEpochCache() } type SessionAttributeStore interface { diff --git a/server/channels/store/storetest/access_control_policy_store.go b/server/channels/store/storetest/access_control_policy_store.go index bf2c0028e692..f9a30eef98c3 100644 --- a/server/channels/store/storetest/access_control_policy_store.go +++ b/server/channels/store/storetest/access_control_policy_store.go @@ -6,6 +6,7 @@ package storetest import ( "errors" "fmt" + "strings" "testing" "github.com/mattermost/mattermost/server/public/model" @@ -35,10 +36,104 @@ func TestAccessControlPolicyStore(t *testing.T, rctx request.CTX, ss store.Store t.Run("SearchByTeamIDWithScope", func(t *testing.T) { testAccessControlPolicyStoreSearchByTeamIDWithScope(t, rctx, ss) }) t.Run("GetActionsForPolicy", func(t *testing.T) { testAccessControlPolicyStoreGetActionsForPolicy(t, rctx, ss) }) t.Run("GetActionsForPolicies", func(t *testing.T) { testAccessControlPolicyStoreGetActionsForPolicies(t, rctx, ss) }) + t.Run("GetEtagEpoch", func(t *testing.T) { testAccessControlPolicyStoreGetEtagEpoch(t, rctx, ss) }) t.Run("PluginPolicy", func(t *testing.T) { testAccessControlPolicyStorePluginPolicy(t, rctx, ss) }) t.Run("TypeImmutableOnSave", func(t *testing.T) { testAccessControlPolicyStoreTypeImmutableOnSave(t, rctx, ss) }) } +func testAccessControlPolicyStoreGetEtagEpoch(t *testing.T, rctx request.CTX, ss store.Store) { + // baseline is the epoch contributed by any system-scoped permission policies already present + // (queried against a channel ID that cannot match anything). Every per-channel assertion is + // made relative to it, so the test stays deterministic regardless of unrelated policies. + baseline, err := ss.AccessControlPolicy().GetEtagEpoch(rctx, model.NewId()) + require.NoError(t, err) + + channelID := model.NewId() + policy := &model.AccessControlPolicy{ + ID: channelID, + Name: "Name", + Type: model.AccessControlPolicyTypeChannel, + Active: true, + Revision: 1, + Version: model.AccessControlPolicyVersionV0_2, + Imports: []string{}, + Rules: []model.AccessControlPolicyRule{ + { + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Expression: "user.properties.program == \"engineering\"", + }, + }, + } + saved, err := ss.AccessControlPolicy().Save(rctx, policy) + require.NoError(t, err) + require.NotZero(t, saved.CreateAt) + t.Cleanup(func() { + require.NoError(t, ss.AccessControlPolicy().Delete(rctx, channelID)) + }) + + t.Run("includes the target channel's own policy row", func(t *testing.T) { + epoch, err := ss.AccessControlPolicy().GetEtagEpoch(rctx, channelID) + require.NoError(t, err) + require.NotEqual(t, baseline, epoch) + require.True(t, strings.HasPrefix(epoch, fmt.Sprintf("%d-", saved.CreateAt))) + }) + + t.Run("a channel policy does not affect a different channel's epoch", func(t *testing.T) { + epoch, err := ss.AccessControlPolicy().GetEtagEpoch(rctx, model.NewId()) + require.NoError(t, err) + require.Equal(t, baseline, epoch) + }) + + t.Run("empty channel id is scoped to permission policies only", func(t *testing.T) { + epoch, err := ss.AccessControlPolicy().GetEtagEpoch(rctx, "") + require.NoError(t, err) + require.Equal(t, baseline, epoch) + }) + + // Guards the case MAX(CreateAt) alone misses. Permission policies matter most here: they are + // system-scoped, so every channel's epoch aggregates all of them. + t.Run("deleting a permission policy that is not the newest still moves the epoch", func(t *testing.T) { + permissionPolicy := func() *model.AccessControlPolicy { + return &model.AccessControlPolicy{ + ID: model.NewId(), + Name: "Permission " + model.NewId(), + Type: model.AccessControlPolicyTypePermission, + Active: true, + Revision: 1, + Version: model.AccessControlPolicyVersionV0_3, + Imports: []string{}, + Roles: []string{model.SystemUserRoleId}, + Rules: []model.AccessControlPolicyRule{ + { + Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, + Expression: "user.properties.program == \"engineering\"", + }, + }, + } + } + + older, err := ss.AccessControlPolicy().Save(rctx, permissionPolicy()) + require.NoError(t, err) + + newer, err := ss.AccessControlPolicy().Save(rctx, permissionPolicy()) + require.NoError(t, err) + require.GreaterOrEqual(t, newer.CreateAt, older.CreateAt) + t.Cleanup(func() { + require.NoError(t, ss.AccessControlPolicy().Delete(rctx, newer.ID)) + }) + + observerChannelID := model.NewId() + before, err := ss.AccessControlPolicy().GetEtagEpoch(rctx, observerChannelID) + require.NoError(t, err) + + require.NoError(t, ss.AccessControlPolicy().Delete(rctx, older.ID)) + + after, err := ss.AccessControlPolicy().GetEtagEpoch(rctx, observerChannelID) + require.NoError(t, err) + require.NotEqual(t, before, after) + }) +} + // testAccessControlPolicyStoreTypeImmutableOnSave pins the invariant the plugin // app layer depends on: a stored policy's Type never changes, so an ownership // decision taken from an earlier read cannot be invalidated by a later save. diff --git a/server/channels/store/storetest/mocks/AccessControlPolicyStore.go b/server/channels/store/storetest/mocks/AccessControlPolicyStore.go index c36c109c1ea7..236b058c8aef 100644 --- a/server/channels/store/storetest/mocks/AccessControlPolicyStore.go +++ b/server/channels/store/storetest/mocks/AccessControlPolicyStore.go @@ -15,6 +15,11 @@ type AccessControlPolicyStore struct { mock.Mock } +// ClearEtagCache provides a mock function with no fields +func (_m *AccessControlPolicyStore) ClearEtagCache() { + _m.Called() +} + // Delete provides a mock function with given fields: rctx, id func (_m *AccessControlPolicyStore) Delete(rctx request.CTX, id string) error { ret := _m.Called(rctx, id) @@ -123,6 +128,34 @@ func (_m *AccessControlPolicyStore) GetActionsForPolicy(rctx request.CTX, policy return r0, r1 } +// GetEtagEpoch provides a mock function with given fields: rctx, channelID +func (_m *AccessControlPolicyStore) GetEtagEpoch(rctx request.CTX, channelID string) (string, error) { + ret := _m.Called(rctx, channelID) + + if len(ret) == 0 { + panic("no return value specified for GetEtagEpoch") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(request.CTX, string) (string, error)); ok { + return rf(rctx, channelID) + } + if rf, ok := ret.Get(0).(func(request.CTX, string) string); ok { + r0 = rf(rctx, channelID) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(request.CTX, string) error); ok { + r1 = rf(rctx, channelID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetPoliciesByFieldID provides a mock function with given fields: rctx, fieldID func (_m *AccessControlPolicyStore) GetPoliciesByFieldID(rctx request.CTX, fieldID string) ([]*model.AccessControlPolicy, error) { ret := _m.Called(rctx, fieldID) @@ -153,6 +186,11 @@ func (_m *AccessControlPolicyStore) GetPoliciesByFieldID(rctx request.CTX, field return r0, r1 } +// InvalidateEtagForChannel provides a mock function with given fields: channelID +func (_m *AccessControlPolicyStore) InvalidateEtagForChannel(channelID string) { + _m.Called(channelID) +} + // Save provides a mock function with given fields: rctx, policy func (_m *AccessControlPolicyStore) Save(rctx request.CTX, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, error) { ret := _m.Called(rctx, policy) diff --git a/server/channels/store/storetest/mocks/AttributesStore.go b/server/channels/store/storetest/mocks/AttributesStore.go index a88283c81b0d..d751e8d76837 100644 --- a/server/channels/store/storetest/mocks/AttributesStore.go +++ b/server/channels/store/storetest/mocks/AttributesStore.go @@ -15,6 +15,11 @@ type AttributesStore struct { mock.Mock } +// ClearUserPropertyValuesEpochCache provides a mock function with no fields +func (_m *AttributesStore) ClearUserPropertyValuesEpochCache() { + _m.Called() +} + // GetChannelMembersToRemove provides a mock function with given fields: rctx, channelID, opts func (_m *AttributesStore) GetChannelMembersToRemove(rctx request.CTX, channelID string, opts model.SubjectSearchOptions) ([]*model.ChannelMember, error) { ret := _m.Called(rctx, channelID, opts) @@ -105,6 +110,39 @@ func (_m *AttributesStore) GetTeamMembersToRemove(rctx request.CTX, teamID strin return r0, r1 } +// GetUserPropertyValuesEpoch provides a mock function with given fields: rctx, userID +func (_m *AttributesStore) GetUserPropertyValuesEpoch(rctx request.CTX, userID string) (string, error) { + ret := _m.Called(rctx, userID) + + if len(ret) == 0 { + panic("no return value specified for GetUserPropertyValuesEpoch") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(request.CTX, string) (string, error)); ok { + return rf(rctx, userID) + } + if rf, ok := ret.Get(0).(func(request.CTX, string) string); ok { + r0 = rf(rctx, userID) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(request.CTX, string) error); ok { + r1 = rf(rctx, userID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// InvalidateUserPropertyValuesEpoch provides a mock function with given fields: userID +func (_m *AttributesStore) InvalidateUserPropertyValuesEpoch(userID string) { + _m.Called(userID) +} + // RefreshAttributes provides a mock function with no fields func (_m *AttributesStore) RefreshAttributes() error { ret := _m.Called() diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 91cd8059bd29..47d9001668ae 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -630,6 +630,21 @@ type TimerLayerWebhookStore struct { Root *TimerLayer } +func (s *TimerLayerAccessControlPolicyStore) ClearEtagCache() { + start := time.Now() + + s.AccessControlPolicyStore.ClearEtagCache() + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if true { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AccessControlPolicyStore.ClearEtagCache", success, elapsed) + } +} + func (s *TimerLayerAccessControlPolicyStore) Delete(rctx request.CTX, id string) error { start := time.Now() @@ -694,6 +709,22 @@ func (s *TimerLayerAccessControlPolicyStore) GetActionsForPolicy(rctx request.CT return result, err } +func (s *TimerLayerAccessControlPolicyStore) GetEtagEpoch(rctx request.CTX, channelID string) (string, error) { + start := time.Now() + + result, err := s.AccessControlPolicyStore.GetEtagEpoch(rctx, channelID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AccessControlPolicyStore.GetEtagEpoch", success, elapsed) + } + return result, err +} + func (s *TimerLayerAccessControlPolicyStore) GetPoliciesByFieldID(rctx request.CTX, fieldID string) ([]*model.AccessControlPolicy, error) { start := time.Now() @@ -710,6 +741,21 @@ func (s *TimerLayerAccessControlPolicyStore) GetPoliciesByFieldID(rctx request.C return result, err } +func (s *TimerLayerAccessControlPolicyStore) InvalidateEtagForChannel(channelID string) { + start := time.Now() + + s.AccessControlPolicyStore.InvalidateEtagForChannel(channelID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if true { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AccessControlPolicyStore.InvalidateEtagForChannel", success, elapsed) + } +} + func (s *TimerLayerAccessControlPolicyStore) Save(rctx request.CTX, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, error) { start := time.Now() @@ -774,6 +820,21 @@ func (s *TimerLayerAccessControlPolicyStore) SetActiveStatusMultiple(rctx reques return result, err } +func (s *TimerLayerAttributesStore) ClearUserPropertyValuesEpochCache() { + start := time.Now() + + s.AttributesStore.ClearUserPropertyValuesEpochCache() + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if true { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AttributesStore.ClearUserPropertyValuesEpochCache", success, elapsed) + } +} + func (s *TimerLayerAttributesStore) GetChannelMembersToRemove(rctx request.CTX, channelID string, opts model.SubjectSearchOptions) ([]*model.ChannelMember, error) { start := time.Now() @@ -822,6 +883,37 @@ func (s *TimerLayerAttributesStore) GetTeamMembersToRemove(rctx request.CTX, tea return result, err } +func (s *TimerLayerAttributesStore) GetUserPropertyValuesEpoch(rctx request.CTX, userID string) (string, error) { + start := time.Now() + + result, err := s.AttributesStore.GetUserPropertyValuesEpoch(rctx, userID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AttributesStore.GetUserPropertyValuesEpoch", success, elapsed) + } + return result, err +} + +func (s *TimerLayerAttributesStore) InvalidateUserPropertyValuesEpoch(userID string) { + start := time.Now() + + s.AttributesStore.InvalidateUserPropertyValuesEpoch(userID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if true { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AttributesStore.InvalidateUserPropertyValuesEpoch", success, elapsed) + } +} + func (s *TimerLayerAttributesStore) RefreshAttributes() error { start := time.Now() diff --git a/server/enterprise/metrics/metrics.go b/server/enterprise/metrics/metrics.go index 6d7773b9e4f3..895533974f1d 100644 --- a/server/enterprise/metrics/metrics.go +++ b/server/enterprise/metrics/metrics.go @@ -551,6 +551,8 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf model.ClusterEventInvalidateCacheForSessionAttributes, model.ClusterEventUpdateSessionAttributes, model.ClusterEventInvalidateCacheForPropertyFields, + model.ClusterEventInvalidateCacheForAccessControlPolicyEtag, + model.ClusterEventInvalidateCacheForUserPropertyValuesEpoch, model.ClusterEventClearSessionCacheForAllUsers, model.ClusterEventInstallPlugin, model.ClusterEventRemovePlugin, diff --git a/server/i18n/en.json b/server/i18n/en.json index afde2a02aea3..67f2caee008e 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -51,6 +51,10 @@ "id": "api.access_control.assign.team_group_constrained", "translation": "A group-synced team cannot have a membership policy applied." }, + { + "id": "api.access_control.decision.unsupported_resource_type.app_error", + "translation": "Render-time access control decisions are not supported for resource type \"{{.Type}}\"." + }, { "id": "api.access_control_policy.channel_permission_policies.feature_disabled", "translation": "Channel-level permission policies feature is not enabled." @@ -5366,6 +5370,14 @@ "id": "app.access_control.plugin.type_conflict.app_error", "translation": "An access control policy of a different type already exists for this ID." }, + { + "id": "app.access_control_decision.subject_mismatch.app_error", + "translation": "Subject does not match the authenticated session user." + }, + { + "id": "app.access_control_decision.unsupported_action.app_error", + "translation": "Unsupported action for render decisions: {{.Action}}" + }, { "id": "app.acknowledgement.batch_save.app_error", "translation": "Failed to save the batch of acknowledgement objects" @@ -11210,6 +11222,26 @@ "id": "model.access.is_valid.user_id.app_error", "translation": "Invalid user id." }, + { + "id": "model.access_control_decision.is_valid.action_empty.app_error", + "translation": "Action names cannot be empty." + }, + { + "id": "model.access_control_decision.is_valid.actions_too_many.app_error", + "translation": "Too many actions requested." + }, + { + "id": "model.access_control_decision.is_valid.resource_id.app_error", + "translation": "Invalid resource id." + }, + { + "id": "model.access_control_decision.is_valid.resource_type.app_error", + "translation": "Resource type is required." + }, + { + "id": "model.access_control_decision.is_valid.subject_id.app_error", + "translation": "Invalid subject id." + }, { "id": "model.access_policy.inherit.already_imported.app_error", "translation": "The parent is already imported." diff --git a/server/public/model/access_control_decision.go b/server/public/model/access_control_decision.go new file mode 100644 index 000000000000..9f6f2d17da07 --- /dev/null +++ b/server/public/model/access_control_decision.go @@ -0,0 +1,114 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "net/http" + "slices" +) + +// maxActionSearchActions bounds how many actions a single Action Search request +// may ask about, to prevent unbounded PDP evaluation loops from a single call. +const maxActionSearchActions = 16 + +// RenderDecisionReasonRestrictedByPolicy is the only denial reason exposed to +// clients. It is intentionally generic: it never reveals policy names, +// expressions, attribute names, or values. +const RenderDecisionReasonRestrictedByPolicy = "restricted_by_policy" + +// RenderPermissionDecision is a non-authoritative, render-time ABAC decision for +// a single action. MUST NOT be used to authorize — enforcement always re-evaluates +// the PDP live on the server. +type RenderPermissionDecision struct { + Allowed bool `json:"allowed"` + Evaluated bool `json:"evaluated"` + Reason string `json:"reason,omitempty"` +} + +// ActionSearchResultAction is the action identity inside an AuthZEN result entry. +type ActionSearchResultAction struct { + Name string `json:"name"` +} + +// ActionSearchResult is the AuthZEN-canonical permitted-action entry. +// Only PERMITTED actions appear in the results list; denial is expressed by omission. +type ActionSearchResult struct { + Action ActionSearchResultAction `json:"action"` +} + +// ActionSearchSubject is RESERVED for Phase 3 cross-subject evaluation. +// In Phase 2, any Subject.ID that does not match the authenticated session user is +// rejected with 403. The field is present in the contract to make Phase 3 a +// non-breaking extension of this endpoint. +type ActionSearchSubject struct { + ID string `json:"id"` + Type string `json:"type,omitempty"` +} + +// ActionSearchPage is RESERVED for Phase 3 pagination. +// Accepted in requests but always ignored; next_token is never emitted in responses. +type ActionSearchPage struct { + NextToken string `json:"next_token,omitempty"` +} + +// ActionSearchRequest asks "for the current session user, on this resource, +// which actions are allowed?". +// +// Actions is optional: +// - nil or empty → discovery mode: the server evaluates all renderable actions +// registered for the resource type and returns the permitted set. +// - non-empty → targeted mode: the server evaluates exactly those actions +// (max 16; all must be registered for the resource type). +// +// Subject is RESERVED. If provided, its ID must equal the authenticated session +// user ID; mismatches are rejected with 403. +// +// Page is RESERVED. Accepted but always ignored. +type ActionSearchRequest struct { + Resource Resource `json:"resource"` + Actions []string `json:"actions,omitempty"` // optional; nil/empty = discovery + Subject *ActionSearchSubject `json:"subject,omitempty"` // reserved + Page *ActionSearchPage `json:"page,omitempty"` // reserved +} + +// ActionSearchResponse returns render-time ABAC decisions for the requested resource. +// +// Results is the AuthZEN-canonical list: only actions the server evaluated as +// PERMITTED appear here. An empty Results ([]) is meaningful — all evaluated actions +// were denied — and is always present (never null). +// +// Decisions is the Mattermost extension: all evaluated actions appear here with full +// decision detail (Allowed, Evaluated, Reason), including denied ones. Always present. +// +// Page is RESERVED; always absent in the current implementation. +type ActionSearchResponse struct { + Resource Resource `json:"resource"` + Results []ActionSearchResult `json:"results"` // no omitempty — [] is meaningful + Decisions map[string]RenderPermissionDecision `json:"decisions"` // no omitempty — {} is meaningful + Page *ActionSearchPage `json:"page,omitempty"` +} + +// IsValid validates the shape of an Action Search request. It does not validate +// that the actions/resource type are supported for rendering; that allowlist +// check happens in the App layer against the renderable-action registry. +func (r *ActionSearchRequest) IsValid() *AppError { + if r.Resource.Type == "" { + return NewAppError("ActionSearchRequest.IsValid", "model.access_control_decision.is_valid.resource_type.app_error", nil, "", http.StatusBadRequest) + } + if !IsValidId(r.Resource.ID) { + return NewAppError("ActionSearchRequest.IsValid", "model.access_control_decision.is_valid.resource_id.app_error", nil, "", http.StatusBadRequest) + } + // nil/empty Actions = discovery mode (valid). Validate bounds only when non-empty. + if len(r.Actions) > maxActionSearchActions { + return NewAppError("ActionSearchRequest.IsValid", "model.access_control_decision.is_valid.actions_too_many.app_error", map[string]any{"Max": maxActionSearchActions}, "", http.StatusBadRequest) + } + if slices.Contains(r.Actions, "") { + return NewAppError("ActionSearchRequest.IsValid", "model.access_control_decision.is_valid.action_empty.app_error", nil, "", http.StatusBadRequest) + } + // Subject shape validation. Identity check (must match session user) is in the app layer. + if r.Subject != nil && !IsValidId(r.Subject.ID) { + return NewAppError("ActionSearchRequest.IsValid", "model.access_control_decision.is_valid.subject_id.app_error", nil, "", http.StatusBadRequest) + } + return nil +} diff --git a/server/public/model/client4.go b/server/public/model/client4.go index bac06c5164b2..9e91cc1c3cec 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -710,6 +710,10 @@ func (c *Client4) accessControlPolicyRoute(policyID string) clientRoute { return c.accessControlPoliciesRoute().Join(url.PathEscape(policyID)) } +func (c *Client4) accessControlDecisionsRoute() clientRoute { + return newClientRoute("access_control").Join("decisions") +} + func (c *Client4) logsRoute() clientRoute { return newClientRoute("logs") } @@ -8405,6 +8409,18 @@ func (c *Client4) SearchAccessControlPolicies(ctx context.Context, options Acces return DecodeJSONFromResponse[*AccessControlPoliciesWithCount](r) } +// SearchAccessControlDecisionActions returns non-authoritative, render-time ABAC +// decisions for the current session user on a resource. Results are for UI +// rendering only; enforcement always re-evaluates the PDP server-side. +func (c *Client4) SearchAccessControlDecisionActions(ctx context.Context, req ActionSearchRequest) (*ActionSearchResponse, *Response, error) { + r, err := c.doAPIPostJSON(ctx, c.accessControlDecisionsRoute().Join("actions", "search"), req) + if err != nil { + return nil, BuildResponse(r), err + } + defer closeBody(r) + return DecodeJSONFromResponse[*ActionSearchResponse](r) +} + func (c *Client4) AssignAccessControlPolicies(ctx context.Context, policyID string, resourceIDs []string) (*Response, error) { var assignments struct { ChannelIds []string `json:"channel_ids"` diff --git a/server/public/model/cluster_message.go b/server/public/model/cluster_message.go index 6cdad684eab2..e5fcc25cdb7d 100644 --- a/server/public/model/cluster_message.go +++ b/server/public/model/cluster_message.go @@ -56,6 +56,8 @@ const ( ClusterEventInvalidateCacheForTermsOfService ClusterEvent = "inv_terms_of_service" ClusterEventInvalidateCacheForUserAutoTranslation ClusterEvent = "inv_user_autotranslation" ClusterEventInvalidateCacheForPostTranslationEtag ClusterEvent = "inv_post_translation_etag" + ClusterEventInvalidateCacheForAccessControlPolicyEtag ClusterEvent = "inv_access_control_policy_etag" + ClusterEventInvalidateCacheForUserPropertyValuesEpoch ClusterEvent = "inv_user_property_values_epoch" ClusterEventAutoTranslationTask ClusterEvent = "autotranslation_task" ClusterEventBusyStateChanged ClusterEvent = "busy_state_change" // Note: if you are adding a new event, please also add it in the slice of diff --git a/server/public/model/websocket_message.go b/server/public/model/websocket_message.go index 2bd42d6fe5e4..265f99e2b63c 100644 --- a/server/public/model/websocket_message.go +++ b/server/public/model/websocket_message.go @@ -89,6 +89,7 @@ const ( WebsocketEventChannelBookmarkDeleted WebsocketEventType = "channel_bookmark_deleted" WebsocketEventChannelBookmarkSorted WebsocketEventType = "channel_bookmark_sorted" WebsocketEventChannelAccessControlUpdated WebsocketEventType = "channel_access_control_updated" + WebsocketEventPermissionPolicyUpdated WebsocketEventType = "permission_policy_updated" WebsocketEventTeamAccessControlUpdated WebsocketEventType = "team_access_control_updated" WebsocketPresenceIndicator WebsocketEventType = "presence" WebsocketPostedNotifyAck WebsocketEventType = "posted_notify_ack" diff --git a/tools/sharedchannel-test/go.mod b/tools/sharedchannel-test/go.mod index 2439f9b13c73..732d47754638 100644 --- a/tools/sharedchannel-test/go.mod +++ b/tools/sharedchannel-test/go.mod @@ -10,7 +10,7 @@ require ( github.com/dyatlov/go-opengraph/opengraph v0.0.0-20220524092352-606d7b1e5f8a // indirect github.com/fatih/color v1.19.0 // indirect github.com/francoispqt/gojay v1.2.13 // indirect - github.com/go-asn1-ber/asn1-ber v1.5.7 // indirect + github.com/go-asn1-ber/asn1-ber v1.5.8 // indirect github.com/goccy/go-yaml v1.19.2 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/uuid v1.6.0 // indirect @@ -23,28 +23,28 @@ require ( github.com/mattermost/go-i18n v1.11.1-0.20211013152124-5c415071e404 // indirect github.com/mattermost/ldap v0.0.0-20231116144001-0f480c025956 // indirect github.com/mattermost/logr/v2 v2.0.22 // indirect - github.com/mattn/go-colorable v0.1.14 // indirect - github.com/mattn/go-isatty v0.0.22 // indirect + github.com/mattn/go-colorable v0.1.15 // indirect + github.com/mattn/go-isatty v0.0.24 // indirect github.com/oklog/run v1.2.0 // indirect github.com/pborman/uuid v1.2.1 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/philhofer/fwd v1.2.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/stretchr/testify v1.11.1 // indirect + github.com/stretchr/testify v1.12.1 // indirect github.com/tinylib/msgp v1.6.4 // indirect github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect github.com/wiggin77/merror v1.0.5 // indirect github.com/wiggin77/srslog v1.0.1 // indirect - golang.org/x/crypto v0.51.0 // indirect - golang.org/x/mod v0.36.0 // indirect - golang.org/x/net v0.54.0 // indirect - golang.org/x/sys v0.44.0 // indirect - golang.org/x/text v0.37.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260519071638-aa98bba5eb94 // indirect - google.golang.org/grpc v1.81.1 // indirect - google.golang.org/protobuf v1.36.11 // indirect + golang.org/x/crypto v0.55.0 // indirect + golang.org/x/mod v0.40.0 // indirect + golang.org/x/net v0.58.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/text v0.41.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260819154853-08b0e4226688 // indirect + google.golang.org/grpc v1.83.1 // indirect + google.golang.org/protobuf v1.36.12 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/tools/sharedchannel-test/go.sum b/tools/sharedchannel-test/go.sum index 0590ac65bc55..00f944729ebe 100644 --- a/tools/sharedchannel-test/go.sum +++ b/tools/sharedchannel-test/go.sum @@ -38,6 +38,7 @@ github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeME github.com/gliderlabs/ssh v0.1.1/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0= github.com/go-asn1-ber/asn1-ber v1.5.7 h1:DTX+lbVTWaTw1hQ+PbZPlnDZPEIs0SS/GCZAl535dDk= github.com/go-asn1-ber/asn1-ber v1.5.7/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0= +github.com/go-asn1-ber/asn1-ber v1.5.8/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0= github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -105,10 +106,12 @@ github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-colorable v0.1.15/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-isatty v0.0.22 h1:j8l17JJ9i6VGPUFUYoTUKPSgKe/83EYU2zBC7YNKMw4= github.com/mattn/go-isatty v0.0.22/go.mod h1:ZXfXG4SQHsB/w3ZeOYbR0PrPwLy+n6xiMrJlRFqopa4= +github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/microcosm-cc/bluemonday v1.0.1/go.mod h1:hsXNsILzKxV+sX77C5b8FSuKF00vh2OMYv+xgHpAMF4= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -167,6 +170,7 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= github.com/tarm/serial v0.0.0-20180830185346-98f6abe2eb07/go.mod h1:kDXzergiv9cbyO7IOYJZWg1U88JhDg3PB6klq9Hg2pA= github.com/tinylib/msgp v1.6.4 h1:mOwYbyYDLPj35mkA2BjjYejgJk9BuHxDdvRnb6v2ZcQ= github.com/tinylib/msgp v1.6.4/go.mod h1:RSp0LW9oSxFut3KzESt5Voq4GVWyS+PSulT77roAqEA= @@ -200,12 +204,14 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= +golang.org/x/mod v0.40.0/go.mod h1:0/weTWkPWGBikyTWAX3dkjVztMmBA5hM0DH6BElSupE= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -217,6 +223,7 @@ golang.org/x/net v0.0.0-20190313220215-9f648a60d977/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -240,12 +247,14 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -269,14 +278,17 @@ google.golang.org/genproto v0.0.0-20181202183823-bd91e49a0898/go.mod h1:7Ep/1NZk google.golang.org/genproto v0.0.0-20190306203927-b5d61aea6440/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= google.golang.org/genproto/googleapis/rpc v0.0.0-20260519071638-aa98bba5eb94 h1:eZCjr/aAF8c5ccm5pb6T4EXgIei5MlAAPWPJk+5ArfY= google.golang.org/genproto/googleapis/rpc v0.0.0-20260519071638-aa98bba5eb94/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260819154853-08b0e4226688/go.mod h1:DjtHYE8FKJLivXcBEjGwndXfIC23G0VpXiXKqG179uA= google.golang.org/grpc v1.14.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw= google.golang.org/grpc v1.16.0/go.mod h1:0JHn/cJsOMiMfNA9+DeHDlAU7KAAB5GDlYFpa9MZMio= google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.81.1 h1:VnnIIZ88UzOOKLukQi+ImGz8O1Wdp8nAGGnvOfEIWQQ= google.golang.org/grpc v1.81.1/go.mod h1:xGH9GfzOyMTGIOXBJmXt+BX/V0kcdQbdcuwQ/zNw42I= +google.golang.org/grpc v1.83.1/go.mod h1:kDyl6SKsiHKt0uylY5gtn5cEjkrIOhQOGDgIc4JGwzQ= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= +google.golang.org/protobuf v1.36.12/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= diff --git a/webapp/channels/src/actions/websocket_actions.test.jsx b/webapp/channels/src/actions/websocket_actions.test.jsx index 0db337b69931..edca3eb3c1d5 100644 --- a/webapp/channels/src/actions/websocket_actions.test.jsx +++ b/webapp/channels/src/actions/websocket_actions.test.jsx @@ -5,7 +5,7 @@ import cloneDeep from 'lodash/cloneDeep'; import {WebSocketEvents} from '@mattermost/client'; -import {ChannelTypes, CloudTypes, JobTypes, TeamTypes} from 'mattermost-redux/action_types'; +import {ChannelTypes, CloudTypes, JobTypes, PostTypes, RenderPermissionTypes, TeamTypes} from 'mattermost-redux/action_types'; import {fetchMyCategories} from 'mattermost-redux/actions/channel_categories'; import {fetchAllMyTeamsChannels, getChannelMember} from 'mattermost-redux/actions/channels'; import {getCustomProfileAttributeFields} from 'mattermost-redux/actions/general'; @@ -40,6 +40,7 @@ import {setIntl} from 'utils/i18n'; import { handleChannelUpdatedEvent, handleChannelAccessControlUpdatedEvent, + handlePermissionPolicyUpdatedEvent, handleTeamAccessControlUpdatedEvent, handleEvent, handleFileUploadRejected, @@ -70,6 +71,7 @@ jest.mock('mattermost-redux/actions/posts', () => ({ getPostThreads: jest.fn(() => ({type: 'GET_THREADS_FOR_POSTS'})), getPostsAround: jest.fn(() => ({type: 'GET_POSTS_AROUND'})), getMentionsAndStatusesForPosts: jest.fn(), + resetReloadPostsInChannel: jest.fn((channelId) => ({type: 'MOCK_RESET_POSTS', channelId})), })); jest.mock('mattermost-redux/actions/channel_categories', () => ({ @@ -123,6 +125,7 @@ jest.mock('actions/global_actions', () => ({ jest.mock('actions/views/channel', () => ({ ...jest.requireActual('actions/views/channel'), syncPostsInChannel: jest.fn(), + loadUnreads: jest.fn((channelId) => ({type: 'MOCK_LOAD_UNREADS', channelId})), })); jest.mock('plugins', () => ({ @@ -1097,12 +1100,20 @@ describe('handleChannelUpdatedEvent', () => { }); describe('handleChannelAccessControlUpdatedEvent', () => { + const withPermissionPolicies = (entities = {}) => ({ + entities: { + general: {config: {FeatureFlagPermissionPolicies: 'true'}}, + ...entities, + }, + }); + beforeEach(() => { invalidateAccessControlAttributesCache.mockClear(); }); test('dispatches RECEIVED_CHANNEL with parsed channel and invalidates attribute cache', () => { - const testStore = configureStore({}); + // Current channel differs from the updated channel, so no post reconciliation. + const testStore = configureStore(withPermissionPolicies({channels: {currentChannelId: 'other-channel'}})); const channel = { id: 'channel-ac-1', team_id: 'team-1', @@ -1116,16 +1127,51 @@ describe('handleChannelAccessControlUpdatedEvent', () => { testStore.dispatch(handleChannelAccessControlUpdatedEvent(msg)); + // Scoped to the one channel: no broad channel/member/team refetch. expect(testStore.getActions()).toEqual([ { type: ChannelTypes.RECEIVED_CHANNEL, data: channel, }, + { + type: RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL, + data: {channelId: 'channel-ac-1', generation: expect.any(Number)}, + }, + { + type: PostTypes.RESET_POSTS_IN_CHANNEL, + channelId: 'channel-ac-1', + }, ]); expect(invalidateAccessControlAttributesCache).toHaveBeenCalledTimes(1); expect(invalidateAccessControlAttributesCache).toHaveBeenCalledWith('channel', 'channel-ac-1'); }); + test('refetches the posts when the updated channel is the one being viewed', () => { + const testStore = configureStore(withPermissionPolicies({channels: {currentChannelId: 'channel-ac-1'}})); + const channel = {id: 'channel-ac-1', team_id: 'team-1', policy_enforced: true}; + + testStore.dispatch(handleChannelAccessControlUpdatedEvent({data: {channel: JSON.stringify(channel)}})); + + // Dropping the chunks alone would leave the channel in view empty until it remounts. + expect(testStore.getActions()).toContainEqual({type: PostTypes.RESET_POSTS_IN_CHANNEL, channelId: 'channel-ac-1'}); + expect(testStore.getActions()).toContainEqual({type: 'MOCK_LOAD_UNREADS', channelId: 'channel-ac-1'}); + }); + + test('updates the channel but drops no render state when permission policies are disabled', () => { + const testStore = configureStore({ + entities: { + general: {config: {FeatureFlagPermissionPolicies: 'false'}}, + channels: {currentChannelId: 'channel-ac-1'}, + }, + }); + const channel = {id: 'channel-ac-1', team_id: 'team-1', policy_enforced: true}; + + testStore.dispatch(handleChannelAccessControlUpdatedEvent({data: {channel: JSON.stringify(channel)}})); + + const types = testStore.getActions().map((a) => a.type); + expect(types).toEqual([ChannelTypes.RECEIVED_CHANNEL]); + }); + test('returns early when msg.data.channel is missing', () => { const testStore = configureStore({}); const msg = {data: {}}; @@ -1681,6 +1727,90 @@ describe('handleCustomAttributeValuesUpdated', () => { }); }); +describe('render permission invalidation via existing events', () => { + const currentUserId = 'user1'; + + function stateWithCurrentUser(currentChannelId = '') { + return { + entities: { + users: { + currentUserId, + profiles: {user1: {id: currentUserId, roles: 'system_user'}}, + }, + channels: {currentChannelId}, + general: {config: {FeatureFlagPermissionPolicies: 'true'}}, + posts: {postsInChannel: {}}, + }, + }; + } + + test('CPA value update for the current user clears render decisions, resets every channel and refetches the visible one', () => { + const testStore = configureStore(stateWithCurrentUser('visible-channel')); + + testStore.dispatch(handleCustomAttributeValuesUpdated({data: {user_id: currentUserId, values: {field1: 'v'}}})); + + const actions = testStore.getActions(); + expect(actions.map((a) => a.type)).toContain(RenderPermissionTypes.CLEAR_RENDER_DECISIONS); + + // One bulk reset (no channel id) rather than one dispatch per loaded channel. + expect(actions).toContainEqual({type: PostTypes.RESET_POSTS_IN_CHANNEL, channelId: undefined}); + expect(actions).toContainEqual({type: 'MOCK_LOAD_UNREADS', channelId: 'visible-channel'}); + }); + + test('CPA value update with no channel in view resets without refetching', () => { + const testStore = configureStore(stateWithCurrentUser()); + + testStore.dispatch(handleCustomAttributeValuesUpdated({data: {user_id: currentUserId, values: {field1: 'v'}}})); + + const types = testStore.getActions().map((a) => a.type); + expect(types).toContain(PostTypes.RESET_POSTS_IN_CHANNEL); + expect(types).not.toContain('MOCK_LOAD_UNREADS'); + }); + + test('CPA value update for another user does NOT invalidate current-user render decisions', () => { + const testStore = configureStore(stateWithCurrentUser()); + + testStore.dispatch(handleCustomAttributeValuesUpdated({data: {user_id: 'someoneElse', values: {field1: 'v'}}})); + + const types = testStore.getActions().map((a) => a.type); + expect(types).not.toContain(RenderPermissionTypes.CLEAR_RENDER_DECISIONS); + expect(types).not.toContain(PostTypes.RESET_POSTS_IN_CHANNEL); + }); +}); + +describe('handlePermissionPolicyUpdatedEvent', () => { + function stateWith(currentChannelId, channelIds = [], permissionPoliciesEnabled = true) { + return { + entities: { + channels: {currentChannelId}, + general: {config: {FeatureFlagPermissionPolicies: permissionPoliciesEnabled ? 'true' : 'false'}}, + posts: {postsInChannel: channelIds.reduce((acc, id) => ({...acc, [id]: []}), {})}, + }, + }; + } + + test('clears render decisions and resets every loaded channel in a single action', () => { + const testStore = configureStore(stateWith('visible-channel', ['visible-channel', 'other-channel'])); + + testStore.dispatch(handlePermissionPolicyUpdatedEvent()); + + const actions = testStore.getActions(); + expect(actions.map((a) => a.type)).toContain(RenderPermissionTypes.CLEAR_RENDER_DECISIONS); + + const resets = actions.filter((a) => a.type === PostTypes.RESET_POSTS_IN_CHANNEL); + expect(resets).toEqual([{type: PostTypes.RESET_POSTS_IN_CHANNEL, channelId: undefined}]); + expect(actions).toContainEqual({type: 'MOCK_LOAD_UNREADS', channelId: 'visible-channel'}); + }); + + test('does nothing when permission policies are disabled', () => { + const testStore = configureStore(stateWith('visible-channel', ['visible-channel'], false)); + + testStore.dispatch(handlePermissionPolicyUpdatedEvent()); + + expect(testStore.getActions()).toEqual([]); + }); +}); + describe('handleCustomAttributeCRUD', () => { const field1 = {id: 'field1', groupid: 'group1', name: 'FIELD ONE', type: 'text'}; const field2 = {id: 'field2', groupid: 'group1', name: 'FIELD TWO', type: 'text'}; diff --git a/webapp/channels/src/actions/websocket_actions.ts b/webapp/channels/src/actions/websocket_actions.ts index 850139cbf23f..b0c7a571214d 100644 --- a/webapp/channels/src/actions/websocket_actions.ts +++ b/webapp/channels/src/actions/websocket_actions.ts @@ -83,6 +83,7 @@ import { fetchSystemPropertyValues, } from 'mattermost-redux/actions/properties'; import {getRecap} from 'mattermost-redux/actions/recaps'; +import {invalidateRenderDecisionsForChannel, clearRenderDecisions} from 'mattermost-redux/actions/render_permissions'; import {loadRolesIfNeeded} from 'mattermost-redux/actions/roles'; import {fetchTeamScheduledPosts} from 'mattermost-redux/actions/scheduled_posts'; import {fetchChannelRemotes} from 'mattermost-redux/actions/shared_channels'; @@ -117,7 +118,7 @@ import { hasAutotranslationBecomeEnabled, } from 'mattermost-redux/selectors/entities/channels'; import {getIsUserStatusesConfigEnabled} from 'mattermost-redux/selectors/entities/common'; -import {getConfig, getFeatureFlagValue, getLicense} from 'mattermost-redux/selectors/entities/general'; +import {getConfig, getFeatureFlagValue, getLicense, isPermissionPoliciesEnabled} from 'mattermost-redux/selectors/entities/general'; import {getGroup} from 'mattermost-redux/selectors/entities/groups'; import {getPost, getMostRecentPostIdInChannel, getTeamIdFromPost} from 'mattermost-redux/selectors/entities/posts'; import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; @@ -148,7 +149,7 @@ import {handleNewPost} from 'actions/post_actions'; import * as StatusActions from 'actions/status_actions'; import {setGlobalItem} from 'actions/storage'; import {loadProfilesForDM, loadProfilesForGM, loadProfilesForSidebar} from 'actions/user_actions'; -import {syncPostsInChannel} from 'actions/views/channel'; +import {loadUnreads, syncPostsInChannel} from 'actions/views/channel'; import {setGlobalDraft, transformServerDraft} from 'actions/views/drafts'; import {openModal, closeModal} from 'actions/views/modals'; import {closeRightHandSide} from 'actions/views/rhs'; @@ -565,6 +566,10 @@ export function handleEvent(msg: WebSocketMessage) { dispatch(handleChannelAccessControlUpdatedEvent(msg)); break; + case WebSocketEvents.PermissionPolicyUpdated: + dispatch(handlePermissionPolicyUpdatedEvent()); + break; + case WebSocketEvents.TeamAccessControlUpdated: dispatch(handleTeamAccessControlUpdatedEvent(msg)); break; @@ -885,7 +890,7 @@ export function handleChannelUpdatedEvent(msg: WebSocketMessages.ChannelUpdated) } export function handleChannelAccessControlUpdatedEvent(msg: WebSocketMessages.ChannelAccessControlUpdated): ThunkActionFunc { - return (doDispatch) => { + return (doDispatch, doGetState) => { if (!msg.data.channel) { return; } @@ -900,6 +905,44 @@ export function handleChannelAccessControlUpdatedEvent(msg: WebSocketMessages.Ch // consumers (e.g. the channel invite modal banner) refetch the // latest attribute set after a policy change. invalidateAccessControlAttributesCache(EntityType.Channel, channel.id); + + // Nothing sanitizes posts or gates affordances without the feature, so no stale render + // state to drop. Matches the CPA and role handlers. + if (!isPermissionPoliciesEnabled(doGetState())) { + return; + } + + doDispatch(invalidateRenderDecisionsForChannel(channel.id)); + doDispatch(refreshPostsAfterPolicyChange(channel.id)); + }; +} + +// Posts already loaded were sanitized under the old policy. Off-screen channels just lose their +// chunks and reload on next visit; the channel in view is refetched explicitly, rather than via +// resetReloadPostsInChannel's deselect/reselect, which does not reliably remount PostList. +// +// loadUnreads and not loadLatestPosts: the unread endpoint carries no ETag, so it cannot 304 onto +// the metadata the old policy produced. +function refreshPostsAfterPolicyChange(channelId?: string): ThunkActionFunc { + return (doDispatch, doGetState) => { + doDispatch({type: PostTypes.RESET_POSTS_IN_CHANNEL, channelId}); + + const currentChannelId = getCurrentChannelId(doGetState()); + if (currentChannelId && (!channelId || channelId === currentChannelId)) { + doDispatch(loadUnreads(currentChannelId)); + } + }; +} + +// Permission policies are system-scoped, hence no resource ID on the event and no narrower reset. +export function handlePermissionPolicyUpdatedEvent(): ThunkActionFunc { + return (doDispatch, doGetState) => { + if (!isPermissionPoliciesEnabled(doGetState())) { + return; + } + + doDispatch(clearRenderDecisions()); + doDispatch(refreshPostsAfterPolicyChange()); }; } @@ -1805,6 +1848,12 @@ function handleUserRoleUpdated(msg: WebSocketMessages.UserRoleUpdated) { store.dispatch({type: UserTypes.RECEIVED_PROFILE, data: {...user, roles}}); dispatch(loadRolesIfNeeded(newRoles)); + if (msg.data.user_id === getCurrentUserId(store.getState()) && + isPermissionPoliciesEnabled(store.getState())) { + store.dispatch(clearRenderDecisions()); + store.dispatch(refreshPostsAfterPolicyChange()); + } + if (demoted && global.location.pathname.startsWith('/admin_console')) { redirectUserToDefaultTeam(); } @@ -1824,12 +1873,22 @@ function handleConfigChanged(msg: WebSocketMessages.ConfigChanged) { dispatch(resetReloadPostsInTranslatedChannels()); } + // If the permission-policies feature availability changed, cached render + // decisions may no longer be valid; clear them so they are recomputed. + if (currentConfig?.FeatureFlagPermissionPolicies !== newConfig?.FeatureFlagPermissionPolicies) { + store.dispatch(clearRenderDecisions()); + } + store.dispatch({type: GeneralTypes.CLIENT_CONFIG_RECEIVED, data: newConfig}); } function handleLicenseChanged(msg: WebSocketMessages.LicenseChanged) { store.dispatch({type: GeneralTypes.CLIENT_LICENSE_RECEIVED, data: msg.data.license}); + // A license change can flip ABAC availability, so clear cached render + // decisions; they will be recomputed on demand. + store.dispatch(clearRenderDecisions()); + // Refresh server limits when license changes since limits may have changed dispatch(getServerLimits()); } @@ -2353,10 +2412,21 @@ function handleChannelBookmarkSorted(msg: WebSocketMessages.ChannelBookmarkSorte }; } -export function handleCustomAttributeValuesUpdated(msg: WebSocketMessages.CPAValuesUpdated) { - return { - type: UserTypes.RECEIVED_CPA_VALUES, - data: {userID: msg.data.user_id, customAttributeValues: msg.data.values}, +export function handleCustomAttributeValuesUpdated(msg: WebSocketMessages.CPAValuesUpdated): ThunkActionFunc { + return (doDispatch, doGetState) => { + doDispatch({ + type: UserTypes.RECEIVED_CPA_VALUES, + data: {userID: msg.data.user_id, customAttributeValues: msg.data.values}, + }); + + // The current user's attribute values are an input to ABAC evaluation, so + // their render decisions are now stale. Other users' updates do not affect + // the current user's own decisions. + if (msg.data.user_id === getCurrentUserId(doGetState()) && + isPermissionPoliciesEnabled(doGetState())) { + doDispatch(clearRenderDecisions()); + doDispatch(refreshPostsAfterPolicyChange()); + } }; } diff --git a/webapp/channels/src/components/advanced_text_editor/use_upload_files.test.tsx b/webapp/channels/src/components/advanced_text_editor/use_upload_files.test.tsx new file mode 100644 index 000000000000..84065c99b3b5 --- /dev/null +++ b/webapp/channels/src/components/advanced_text_editor/use_upload_files.test.tsx @@ -0,0 +1,78 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type React from 'react'; + +import {renderHookWithContext} from 'tests/react_testing_utils'; +import {TestHelper} from 'utils/test_helper'; + +import type {PostDraft} from 'types/store/draft'; + +import useUploadFiles from './use_upload_files'; + +jest.mock('components/file_upload', () => () => null); +jest.mock('components/file_preview', () => () => null); + +describe('useUploadFiles ABAC upload gate', () => { + const channel = TestHelper.getChannelMock({id: 'channelid1channelid1channelid1', team_id: 'teamid1teamid1teamid1teamid1te'}); + const channelId = channel.id; + const draft = {message: '', fileInfos: [], uploadsInProgress: [], channelId, rootId: ''} as unknown as PostDraft; + + function stateWithDecision(decision?: {allowed: boolean; evaluated: boolean}) { + return { + entities: { + channels: {channels: {[channelId]: channel}}, + general: { + config: {FeatureFlagPermissionPolicies: 'true'}, + license: {}, + }, + renderPermissions: { + byResource: decision ? { + channel: { + [channelId]: { + upload_file_attachment: {...decision, generation: 1}, + }, + }, + } : {}, + }, + }, + }; + } + + function renderUpload(initialState: ReturnType) { + return renderHookWithContext(() => useUploadFiles( + draft, + '', + channelId, + false, + {current: {}}, + false, // isDisabled + {current: null} as any, + jest.fn(), + jest.fn(), + jest.fn(), + false, // isPostBeingEdited + ), initialState); + } + + test('renders the upload control enabled when policy allows upload', () => { + const {result} = renderUpload(stateWithDecision({allowed: true, evaluated: true})); + + const [, fileUploadJSX] = result.current as [unknown, React.ReactElement]; + expect(fileUploadJSX.props.disabledByPolicy).toBe(false); + }); + + test('renders the upload control disabled when policy denies upload', () => { + const {result} = renderUpload(stateWithDecision({allowed: false, evaluated: true})); + + const [, fileUploadJSX] = result.current as [unknown, React.ReactElement]; + expect(fileUploadJSX.props.disabledByPolicy).toBe(true); + }); + + test('renders the upload control enabled while no decision is cached', () => { + const {result} = renderUpload(stateWithDecision()); + + const [, fileUploadJSX] = result.current as [unknown, React.ReactElement]; + expect(fileUploadJSX.props.disabledByPolicy).toBe(false); + }); +}); diff --git a/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx b/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx index 28c3ee0fbf99..1221368da43c 100644 --- a/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx +++ b/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx @@ -15,6 +15,7 @@ import {sortFileInfos} from 'mattermost-redux/utils/file_utils'; import {getCurrentLocale} from 'selectors/i18n'; +import {useRenderPermission} from 'components/common/hooks/useRenderPermission'; import FilePreview from 'components/file_preview'; import type {FilePreviewInfo} from 'components/file_preview/file_preview'; import FileUpload from 'components/file_upload'; @@ -48,6 +49,10 @@ const useUploadFiles = ( }); const editAttachmentsDisabled = isPostBeingEdited && !canEditAttachments; + // Fail open while in flight: the endpoint enforces regardless, and failing closed flickers the + // button disabled on every first visit to a channel. + const uploadAllowedByPolicy = useRenderPermission({resourceType: 'channel', resourceId: channelId, action: 'upload_file_attachment'}, true); + const [uploadsProgressPercent, setUploadsProgressPercent] = useState<{[clientID: string]: FilePreviewInfo}>({}); const fileUploadRef = useRef(null); @@ -191,6 +196,7 @@ const useUploadFiles = ( rootId={postId} channelId={channelId} postType={postType} + disabledByPolicy={!uploadAllowedByPolicy} /> ); diff --git a/webapp/channels/src/components/common/hooks/useRenderPermission.test.tsx b/webapp/channels/src/components/common/hooks/useRenderPermission.test.tsx new file mode 100644 index 000000000000..8b2c85c1a4f7 --- /dev/null +++ b/webapp/channels/src/components/common/hooks/useRenderPermission.test.tsx @@ -0,0 +1,151 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import nock from 'nock'; + +import {Client4} from 'mattermost-redux/client'; + +import {renderHookWithContext, waitFor} from 'tests/react_testing_utils'; + +import {useRenderPermission} from './useRenderPermission'; + +const channelId = 'channelid1channelid1channelid1'; +const args = {resourceType: 'channel', resourceId: channelId, action: 'upload_file_attachment'}; + +function baseState(permissionPoliciesEnabled = true, byResource: any = {}) { + return { + entities: { + general: { + config: {FeatureFlagPermissionPolicies: permissionPoliciesEnabled ? 'true' : 'false'}, + license: {}, + }, + renderPermissions: {byResource}, + }, + }; +} + +function decisionResponse(allowed: boolean) { + return { + resource: {type: 'channel', id: channelId}, + results: allowed ? [{action: {name: 'upload_file_attachment'}}] : [], + decisions: {upload_file_attachment: {allowed, evaluated: true}}, + }; +} + +describe('useRenderPermission', () => { + beforeAll(() => { + Client4.setUrl('http://localhost:8065'); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + test('returns the default and makes no request when permission policies are disabled', () => { + const {result} = renderHookWithContext(() => useRenderPermission(args, true), baseState(false)); + + expect(result.current).toBe(true); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + test('returns the default and makes no request without a resource id', () => { + const {result} = renderHookWithContext(() => useRenderPermission({...args, resourceId: ''}, false), baseState()); + + expect(result.current).toBe(false); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + test('returns the default while the decision is in flight, then the decision', async () => { + const scope = nock(Client4.getBaseRoute()). + post('/access_control/decisions/actions/search'). + reply(200, decisionResponse(false)); + + const {result} = renderHookWithContext(() => useRenderPermission(args, true), baseState()); + + expect(result.current).toBe(true); + + await waitFor(() => { + expect(scope.isDone()).toBe(true); + }); + await waitFor(() => { + expect(result.current).toBe(false); + }); + }); + + test('reads a cached decision without fetching', () => { + const byResource = { + channel: { + [channelId]: { + upload_file_attachment: {allowed: false, evaluated: true, reason: 'restricted_by_policy', generation: 1}, + }, + }, + }; + + const {result} = renderHookWithContext(() => useRenderPermission(args, true), baseState(true, byResource)); + + expect(result.current).toBe(false); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + test('a cached deny does not trigger a refetch loop', () => { + const byResource = { + channel: { + [channelId]: { + upload_file_attachment: {allowed: false, evaluated: true, generation: 1}, + }, + }, + }; + + const {result, rerender} = renderHookWithContext(() => useRenderPermission(args, true), baseState(true, byResource)); + + rerender(); + rerender(); + + expect(result.current).toBe(false); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + test('two hooks on the same resource and action issue a single request', async () => { + const search = jest.spyOn(Client4, 'searchAccessControlDecisionActions'). + mockResolvedValue(decisionResponse(true)); + + const {result} = renderHookWithContext(() => [ + useRenderPermission(args, false), + useRenderPermission(args, false), + ], baseState()); + + await waitFor(() => { + expect(result.current).toEqual([true, true]); + }); + + expect(search).toHaveBeenCalledTimes(1); + expect(search).toHaveBeenCalledWith('channel', channelId, ['upload_file_attachment']); + + search.mockRestore(); + }); + + test('different actions on the same resource are batched into one request', async () => { + const search = jest.spyOn(Client4, 'searchAccessControlDecisionActions').mockResolvedValue({ + resource: {type: 'channel', id: channelId}, + results: [{action: {name: 'upload_file_attachment'}}], + decisions: { + upload_file_attachment: {allowed: true, evaluated: true}, + download_file_attachment: {allowed: false, evaluated: true}, + }, + }); + + const {result} = renderHookWithContext(() => [ + useRenderPermission(args, false), + useRenderPermission({...args, action: 'download_file_attachment'}, true), + ], baseState()); + + await waitFor(() => { + expect(result.current).toEqual([true, false]); + }); + + expect(search).toHaveBeenCalledTimes(1); + expect(search).toHaveBeenCalledWith('channel', channelId, ['upload_file_attachment', 'download_file_attachment']); + + search.mockRestore(); + }); +}); diff --git a/webapp/channels/src/components/common/hooks/useRenderPermission.ts b/webapp/channels/src/components/common/hooks/useRenderPermission.ts new file mode 100644 index 000000000000..1a4b41426e69 --- /dev/null +++ b/webapp/channels/src/components/common/hooks/useRenderPermission.ts @@ -0,0 +1,45 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {useMemo} from 'react'; +import {useSelector} from 'react-redux'; + +import type {RenderDecisionIdentifier, RenderPermissionEntry} from '@mattermost/types/render_permissions'; + +import {fetchRenderActionsForResourceBatched} from 'mattermost-redux/actions/render_permissions'; +import {isPermissionPoliciesEnabled} from 'mattermost-redux/selectors/entities/general'; +import {getRenderDecision} from 'mattermost-redux/selectors/entities/render_permissions'; + +import {makeUseEntity} from './useEntity'; + +// Returns the entry, never its boolean: useEntity reads a falsy entity as not-loaded, so a deny +// would refetch on every render forever. +const useRenderDecision = makeUseEntity({ + name: 'useRenderDecision', + fetch: fetchRenderActionsForResourceBatched, + selector: getRenderDecision, +}); + +// Advisory only — the server re-evaluates on every request, so never gate a real action on this. +// +// defaultAllowed is required rather than defaulted: silently failing open on a permissions +// affordance is the mistake worth making impossible. +export function useRenderPermission({resourceType, resourceId, action}: RenderDecisionIdentifier, defaultAllowed: boolean): boolean { + const enabled = useSelector(isPermissionPoliciesEnabled); + + // Undefined identifier means nothing to decide, and useEntity then neither selects nor fetches. + // A channel-less surface is real: the editor is exported to plugins, which can render it + // without one. Memoized because useEntity keys its fetch effect on the identifier. + const identifier = useMemo( + () => (enabled && resourceId ? {resourceType, resourceId, action} : undefined), + [enabled, resourceType, resourceId, action], + ); + + const decision = useRenderDecision(identifier as RenderDecisionIdentifier); + + if (!identifier || !decision?.evaluated) { + return defaultAllowed; + } + + return decision.allowed; +} diff --git a/webapp/channels/src/components/file_upload/file_upload.test.tsx b/webapp/channels/src/components/file_upload/file_upload.test.tsx index 37160c75f8e0..f3b66bd40966 100644 --- a/webapp/channels/src/components/file_upload/file_upload.test.tsx +++ b/webapp/channels/src/components/file_upload/file_upload.test.tsx @@ -438,6 +438,119 @@ describe('components/FileUpload', () => { expect(baseProps.onFileUploadChange).toHaveBeenCalledWith(); }); + describe('disabledByPolicy', () => { + // jest-dom's toBeDisabled() only recognises the native attribute, and the button uses + // aria-disabled so the tooltip still opens. Playwright's accepts either, so e2e is fine. + test('renders the button visible but disabled, and keeps its id', () => { + const {container} = renderWithContext( + , + ); + + const button = container.querySelector('#fileUploadButton'); + expect(button).toBeVisible(); + expect(button).toHaveAttribute('aria-disabled', 'true'); + }); + + test('does not open the file picker when the disabled button is clicked', () => { + const ref = React.createRef(); + const {container} = renderWithContext( + , + ); + + const click = jest.fn(); + (ref.current!.fileInput as any).current = {click}; + + (container.querySelector('#fileUploadButton') as HTMLButtonElement).click(); + + expect(click).not.toHaveBeenCalled(); + }); + + test('renders nothing when RBAC also denies uploads', () => { + const {container} = renderWithContext( + , + ); + + expect(container.querySelector('#fileUploadButton')).toBeNull(); + }); + + test('keeps the attachment control rendered and disabled when plugins register upload methods', () => { + const pluginMethod = { + id: 'pluginmethodid', + pluginId: 'pluginid', + text: 'Upload from somewhere', + action: jest.fn(), + icon: , + }; + + const {container} = renderWithContext( + , + ); + + const button = container.querySelector('#fileUploadButton'); + expect(button).toBeVisible(); + expect(button).toHaveAttribute('aria-disabled', 'true'); + }); + + test('blocks the drop path, not only the button', () => { + const ref = React.createRef(); + renderWithContext( + , + ); + + const e = {dataTransfer: {files: [{name: 'file1.pdf'}]}} as unknown as DragEvent; + const instance = ref.current!; + instance.uploadFiles = jest.fn(); + instance.handleDrop(e); + + expect(instance.uploadFiles).not.toHaveBeenCalled(); + expect(baseProps.onUploadError).toHaveBeenCalledWith('File uploads are restricted in this channel'); + }); + + test('blocks the paste path, not only the button', () => { + const event = new Event('paste'); + event.preventDefault = jest.fn(); + const getAsFile = jest.fn().mockReturnValue(new File(['test'], 'test.png')); + (event as any).clipboardData = {items: [{getAsFile, kind: 'file', name: 'test.png'}], types: ['image/png'], getData: () => {}}; + + const ref = React.createRef(); + renderWithContext( + , + ); + + const instance = ref.current!; + jest.spyOn(instance, 'containsEventTarget').mockReturnValue(true); + const spy = jest.spyOn(instance, 'checkPluginHooksAndUploadFiles'); + + document.dispatchEvent(event); + + expect(spy).not.toHaveBeenCalled(); + expect(baseProps.onUploadError).toHaveBeenCalledWith('File uploads are restricted in this channel'); + }); + }); + test('FilesWillUploadHook - should reject all files', () => { const pluginHook = () => { return {files: null}; diff --git a/webapp/channels/src/components/file_upload/file_upload.tsx b/webapp/channels/src/components/file_upload/file_upload.tsx index 0706d6540123..24f36cda9994 100644 --- a/webapp/channels/src/components/file_upload/file_upload.tsx +++ b/webapp/channels/src/components/file_upload/file_upload.tsx @@ -140,6 +140,13 @@ export type Props = { */ canUploadFiles: boolean; + /** + * An access control policy denies uploads here. Renders the control disabled with an + * explanation, unlike canUploadFiles which hides it: policy denial is user-specific, so the + * user needs to know why their experience differs from a colleague's in the same channel. + */ + disabledByPolicy?: boolean; + /** * Plugin file upload methods to be added */ @@ -378,6 +385,11 @@ export class FileUpload extends PureComponent { return; } + if (this.props.disabledByPolicy) { + this.props.onUploadError(localizeMessage({id: 'file_upload.disabled_by_policy', defaultMessage: 'File uploads are restricted in this channel'})); + return; + } + this.props.onUploadError(null); const items = e.dataTransfer.items || []; @@ -523,6 +535,11 @@ export class FileUpload extends PureComponent { return; } + if (this.props.disabledByPolicy) { + this.props.onUploadError(this.props.intl.formatMessage({id: 'file_upload.disabled_by_policy', defaultMessage: 'File uploads are restricted in this channel'})); + return; + } + const fileNamePrefixIfNoName = this.props.intl.formatMessage({id: 'file_upload.pasted', defaultMessage: 'Image Pasted at '}); const fileList = fileClipboardItems. @@ -548,6 +565,12 @@ export class FileUpload extends PureComponent { this.props.onUploadError(localizeMessage({id: 'file_upload.disabled', defaultMessage: 'File attachments are disabled.'})); return; } + + if (this.props.disabledByPolicy) { + this.props.onUploadError(localizeMessage({id: 'file_upload.disabled_by_policy', defaultMessage: 'File uploads are restricted in this channel'})); + return; + } + const postTextbox = this.props.postType === 'post' && document.activeElement?.id === 'post_textbox'; const commentTextbox = this.props.postType === 'comment' && document.activeElement?.id === 'reply_textbox'; const threadTextbox = this.props.postType === 'thread' && document.activeElement?.id === 'reply_textbox'; @@ -596,6 +619,11 @@ export class FileUpload extends PureComponent { simulateInputClick = (e: MouseEvent | TouchEvent) => { e.preventDefault(); e.stopPropagation(); + + if (this.props.disabledByPolicy) { + return; + } + this.fileInput.current?.click(); }; @@ -608,35 +636,51 @@ export class FileUpload extends PureComponent { const buttonAriaLabel = formatMessage({id: 'accessibility.button.attachment', defaultMessage: 'attachment'}); const iconAriaLabel = formatMessage({id: 'generic_icons.attach', defaultMessage: 'Attachment Icon'}); - if (this.props.pluginFileUploadMethods.length === 0) { + const {disabledByPolicy} = this.props; + + const dimmed = uploadsRemaining <= 0 || disabledByPolicy; + + const uploadTooltip = disabledByPolicy ? formatMessage({id: 'file_upload.disabled_by_policy', defaultMessage: 'File uploads are restricted in this channel'}) : ( + + ); + + // aria-disabled, not the disabled attribute: a disabled button emits no pointer or focus + // events, so WithTooltip could never show the tooltip explaining the denial — the whole + // reason the control stays visible. Every upload entry point checks disabledByPolicy. + const attachmentButton = (onActivate?: (e: MouseEvent | TouchEvent) => void) => ( + + + + ); + + // No entry points for a denied user: a menu of plugin sources that would each be rejected + // is worse than one control that says why. + if (disabledByPolicy) { + bodyAction =
{attachmentButton()}
; + } else if (this.props.pluginFileUploadMethods.length === 0) { bodyAction = (
- - } - > - - + {attachmentButton(this.simulateInputClick)} { multiple={true} /> - - } - > - - + {attachmentButton()} { } return ( -
+
{bodyAction}
); diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 8ef97de2974e..56f6cc379abc 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5366,6 +5366,7 @@ "file_type.video": "Video", "file_type.word": "Word Document", "file_upload.disabled": "File attachments are disabled.", + "file_upload.disabled_by_policy": "File uploads are restricted in this channel", "file_upload.drag_folder": "This attachment cannot be uploaded.", "file_upload.fileAbove": "File above {max}MB could not be uploaded: {filename}", "file_upload.filesAbove": "Files above {max}MB could not be uploaded: {filenames}", diff --git a/webapp/channels/src/packages/mattermost-redux/src/action_types/index.ts b/webapp/channels/src/packages/mattermost-redux/src/action_types/index.ts index 8677ac4dddde..126faf2e5d62 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/action_types/index.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/action_types/index.ts @@ -28,6 +28,7 @@ import PostTypes from './posts'; import PreferenceTypes from './preferences'; import PropertyTypes from './properties'; import RecapTypes from './recaps'; +import RenderPermissionTypes from './render_permissions'; import RoleTypes from './roles'; import SchemeTypes from './schemes'; import ScheduledPostTypes from './scheudled_posts'; @@ -72,6 +73,7 @@ export { ContentFlaggingTypes, PropertyTypes, AgentTypes, + RenderPermissionTypes, WebSocketTypes, }; diff --git a/webapp/channels/src/packages/mattermost-redux/src/action_types/render_permissions.ts b/webapp/channels/src/packages/mattermost-redux/src/action_types/render_permissions.ts new file mode 100644 index 000000000000..1e53e4d3f77a --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/action_types/render_permissions.ts @@ -0,0 +1,10 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import keyMirror from 'mattermost-redux/utils/key_mirror'; + +export default keyMirror({ + RECEIVED_RENDER_DECISIONS: null, + INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL: null, + CLEAR_RENDER_DECISIONS: null, +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/render_permissions.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/render_permissions.ts new file mode 100644 index 000000000000..77b95d7c8793 --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/render_permissions.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {ActionSearchResponse, RenderDecisionIdentifier} from '@mattermost/types/render_permissions'; + +import {RenderPermissionTypes} from 'mattermost-redux/action_types'; +import {forceLogoutIfNecessary} from 'mattermost-redux/actions/helpers'; +import {Client4} from 'mattermost-redux/client'; +import type {ActionFuncAsync} from 'mattermost-redux/types/actions'; +import {DelayedDataLoader} from 'mattermost-redux/utils/data_loader'; + +// Ordering identity for fetches and invalidations, so the reducer can drop superseded completions. +// A counter rather than Date.now(), which collides within a millisecond. +let generationCounter = 0; + +export function fetchRenderActionsForResource(resourceType: string, resourceId: string, actions: string[]): ActionFuncAsync { + return async (dispatch, getState) => { + const generation = ++generationCounter; + + let data: ActionSearchResponse; + try { + data = await Client4.searchAccessControlDecisionActions(resourceType, resourceId, actions); + } catch (error) { + forceLogoutIfNecessary(error, dispatch, getState); + return {error}; + } + + dispatch({ + type: RenderPermissionTypes.RECEIVED_RENDER_DECISIONS, + data: { + resourceType: data.resource.type, + resourceId: data.resource.id, + actions: data.decisions, + generation, + }, + }); + + return {data}; + }; +} + +// Matches the server's per-request action cap. Safe for the grouping below, since a group can +// never hold more identifiers than the batch itself. +const maxRenderDecisionsPerBatch = 16; +const renderDecisionsBatchWaitMs = 100; + +function sameRenderDecisionIdentifier(a: RenderDecisionIdentifier, b: RenderDecisionIdentifier) { + return a.resourceType === b.resourceType && a.resourceId === b.resourceId && a.action === b.action; +} + +// Coalesces the fetches of components unaware of each other — centre channel and RHS editors +// today, one hook per post or attachment once downloads land. +export function fetchRenderActionsForResourceBatched(identifier: RenderDecisionIdentifier): ActionFuncAsync { + return async (dispatch, getState, {loaders}: any) => { + if (!loaders.renderDecisionsLoader) { + loaders.renderDecisionsLoader = new DelayedDataLoader({ + + // Action Search takes one resource per request, so a batch becomes one request per + // resource carrying that resource's actions. + fetchBatch: (identifiers) => { + const byResource = new Map(); + + for (const queued of identifiers) { + const key = `${queued.resourceType}:${queued.resourceId}`; + const group = byResource.get(key); + if (group) { + group.actions.push(queued.action); + } else { + byResource.set(key, {resourceType: queued.resourceType, resourceId: queued.resourceId, actions: [queued.action]}); + } + } + + return Promise.all(Array.from(byResource.values()).map(({resourceType, resourceId, actions}) => + dispatch(fetchRenderActionsForResource(resourceType, resourceId, actions)))); + }, + maxBatchSize: maxRenderDecisionsPerBatch, + wait: renderDecisionsBatchWaitMs, + comparator: sameRenderDecisionIdentifier, + }); + } + + const loader = loaders.renderDecisionsLoader as DelayedDataLoader; + loader.queue([identifier]); + + return {data: true}; + }; +} + +// Stamped with the current generation so the reducer can discard fetches already in flight. +export function invalidateRenderDecisionsForChannel(channelId: string) { + return {type: RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL, data: {channelId, generation: generationCounter}}; +} + +export function clearRenderDecisions() { + return {type: RenderPermissionTypes.CLEAR_RENDER_DECISIONS, data: {generation: generationCounter}}; +} diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/index.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/index.ts index 777ad6845612..bb80e0ac0808 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/index.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/index.ts @@ -24,6 +24,7 @@ import posts from './posts'; import preferences from './preferences'; import properties from './properties'; import recaps from './recaps'; +import renderPermissions from './render_permissions'; import roles from './roles'; import scheduledPosts from './scheduled_posts'; import schemes from './schemes'; @@ -67,4 +68,5 @@ export default combineReducers({ sharedChannels, contentFlagging, properties, + renderPermissions, }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/render_permissions.test.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/render_permissions.test.ts new file mode 100644 index 000000000000..582cfd334b8f --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/render_permissions.test.ts @@ -0,0 +1,103 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {RenderPermissionsState} from '@mattermost/types/render_permissions'; + +import {RenderPermissionTypes, UserTypes} from 'mattermost-redux/action_types'; + +import reducer from './render_permissions'; + +describe('reducers.entities.renderPermissions', () => { + const received = (resourceId: string, actions: Record, generation: number) => ({ + type: RenderPermissionTypes.RECEIVED_RENDER_DECISIONS, + data: {resourceType: 'channel', resourceId, actions, generation}, + }); + + test('RECEIVED_RENDER_DECISIONS stores decisions by resource/action', () => { + const state = reducer(undefined, received('chan1', {upload_file_attachment: {allowed: false, evaluated: true}}, 1)); + expect(state.byResource.channel.chan1.upload_file_attachment).toEqual({ + allowed: false, + evaluated: true, + generation: 1, + }); + }); + + test('newer generation overwrites, stale generation is ignored', () => { + let state = reducer(undefined, received('chan1', {upload_file_attachment: {allowed: false, evaluated: true}}, 5)); + + // Newer generation wins. + state = reducer(state, received('chan1', {upload_file_attachment: {allowed: true, evaluated: true}}, 6)); + expect(state.byResource.channel.chan1.upload_file_attachment.allowed).toBe(true); + expect(state.byResource.channel.chan1.upload_file_attachment.generation).toBe(6); + + // Stale completion (older generation) must NOT overwrite. + const before = state; + state = reducer(state, received('chan1', {upload_file_attachment: {allowed: false, evaluated: true}}, 4)); + expect(state.byResource.channel.chan1.upload_file_attachment.allowed).toBe(true); + expect(state.byResource.channel.chan1.upload_file_attachment.generation).toBe(6); + expect(state).toBe(before); // unchanged reference when nothing applied + }); + + test('INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL drops only that channel', () => { + let state = reducer(undefined, received('chan1', {upload_file_attachment: {allowed: true, evaluated: true}}, 1)); + state = reducer(state, received('chan2', {upload_file_attachment: {allowed: true, evaluated: true}}, 2)); + + state = reducer(state, {type: RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL, data: {channelId: 'chan1', generation: 2}}); + expect(state.byResource.channel.chan1).toBeUndefined(); + expect(state.byResource.channel.chan2).toBeDefined(); + }); + + test('CLEAR_RENDER_DECISIONS and LOGOUT_SUCCESS reset to initial', () => { + const seeded: RenderPermissionsState = { + byResource: {channel: {chan1: {upload_file_attachment: {allowed: true, evaluated: true, generation: 1}}}}, + invalidatedAt: 0, + invalidatedAtByResource: {}, + }; + + expect(reducer(seeded, {type: RenderPermissionTypes.CLEAR_RENDER_DECISIONS, data: {generation: 1}}).byResource).toEqual({}); + expect(reducer(seeded, {type: UserTypes.LOGOUT_SUCCESS, data: {}}).byResource).toEqual({}); + }); + + describe('a completion that lands after an invalidation is discarded', () => { + test('after a clear', () => { + let state = reducer(undefined, {type: RenderPermissionTypes.CLEAR_RENDER_DECISIONS, data: {generation: 5}}); + + // The in-flight fetch was dispatched before the clear, so it carries an older generation. + state = reducer(state, received('chan1', {upload_file_attachment: {allowed: true, evaluated: true}}, 5)); + expect(state.byResource).toEqual({}); + + // A fetch started after the clear repopulates it. + state = reducer(state, received('chan1', {upload_file_attachment: {allowed: true, evaluated: true}}, 6)); + expect(state.byResource.channel.chan1.upload_file_attachment.allowed).toBe(true); + }); + + test('after a channel invalidation', () => { + let state = reducer(undefined, {type: RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL, data: {channelId: 'chan1', generation: 5}}); + + state = reducer(state, received('chan1', {upload_file_attachment: {allowed: true, evaluated: true}}, 5)); + expect(state.byResource).toEqual({}); + + state = reducer(state, received('chan1', {upload_file_attachment: {allowed: false, evaluated: true}}, 6)); + expect(state.byResource.channel.chan1.upload_file_attachment.allowed).toBe(false); + }); + + test('but a channel invalidation does not discard another channel in flight', () => { + let state = reducer(undefined, {type: RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL, data: {channelId: 'chan1', generation: 5}}); + + // chan2's fetch was issued before the invalidation of chan1 but is unaffected by it. + state = reducer(state, received('chan2', {upload_file_attachment: {allowed: true, evaluated: true}}, 5)); + expect(state.byResource.channel.chan2.upload_file_attachment.allowed).toBe(true); + expect(state.byResource.channel.chan1).toBeUndefined(); + }); + + test('tolerating state rehydrated from before the invalidation stamps existed', () => { + const rehydrated = {byResource: {}} as RenderPermissionsState; + + const received1 = reducer(rehydrated, received('chan1', {upload_file_attachment: {allowed: true, evaluated: true}}, 1)); + expect(received1.byResource.channel.chan1.upload_file_attachment.allowed).toBe(true); + + const invalidated = reducer(rehydrated, {type: RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL, data: {channelId: 'chan1', generation: 1}}); + expect(invalidated.invalidatedAtByResource.channel.chan1).toBe(1); + }); + }); +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/render_permissions.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/render_permissions.ts new file mode 100644 index 000000000000..a91e09614c18 --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/render_permissions.ts @@ -0,0 +1,144 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {RenderPermissionsState, RenderPermissionEntry} from '@mattermost/types/render_permissions'; + +import {RenderPermissionTypes, UserTypes} from 'mattermost-redux/action_types'; +import type {MMReduxAction} from 'mattermost-redux/action_types'; + +// Matches the server's AccessControlPolicyTypeChannel. +const CHANNEL_RESOURCE_TYPE = 'channel'; + +type ByResource = RenderPermissionsState['byResource']; +type InvalidatedAtByResource = RenderPermissionsState['invalidatedAtByResource']; + +export function invalidatedAt(state = 0, action: MMReduxAction): number { + switch (action.type) { + case RenderPermissionTypes.CLEAR_RENDER_DECISIONS: + return (action.data?.generation as number) ?? state; + default: + return state; + } +} + +// Dropped wholesale on a cache-wide invalidation, whose stamp already supersedes every entry here. +export function invalidatedAtByResource(state: InvalidatedAtByResource = {}, action: MMReduxAction): InvalidatedAtByResource { + switch (action.type) { + case RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL: { + const {channelId, generation} = action.data as {channelId: string; generation: number}; + return { + ...state, + [CHANNEL_RESOURCE_TYPE]: { + ...state[CHANNEL_RESOURCE_TYPE], + [channelId]: generation, + }, + }; + } + case RenderPermissionTypes.CLEAR_RENDER_DECISIONS: + case UserTypes.LOGOUT_SUCCESS: + return {}; + default: + return state; + } +} + +// The one key that can't be reduced in isolation: applying a completion depends on the stamps. +export function byResource( + state: ByResource = {}, + action: MMReduxAction, + nextInvalidatedAt: number, + nextInvalidatedAtByResource: InvalidatedAtByResource, +): ByResource { + switch (action.type) { + case RenderPermissionTypes.RECEIVED_RENDER_DECISIONS: { + const {resourceType, resourceId, actions, generation} = action.data as { + resourceType: string; + resourceId: string; + actions: Record; + generation: number; + }; + + // A fetch in flight when an invalidation landed predates it and must not repopulate the + // cache; the per-entry check below can't catch that, since the entry is gone. Scoped stamp + // consulted too, so invalidating one channel doesn't discard another's in-flight fetch. + const invalidated = Math.max( + nextInvalidatedAt, + nextInvalidatedAtByResource[resourceType]?.[resourceId] ?? 0, + ); + if (generation <= invalidated) { + return state; + } + + const existingForType = state[resourceType] ?? {}; + const existingForResource = existingForType[resourceId] ?? {}; + + const nextForResource: {[action: string]: RenderPermissionEntry} = {...existingForResource}; + let changed = false; + for (const [actionName, decision] of Object.entries(actions)) { + const prev = nextForResource[actionName]; + + if (prev && prev.generation > generation) { + continue; + } + nextForResource[actionName] = { + ...decision, + generation, + }; + changed = true; + } + + if (!changed) { + return state; + } + + return { + ...state, + [resourceType]: { + ...existingForType, + [resourceId]: nextForResource, + }, + }; + } + case RenderPermissionTypes.INVALIDATE_RENDER_DECISIONS_FOR_CHANNEL: { + const channelId = action.data.channelId as string; + const channels = state[CHANNEL_RESOURCE_TYPE]; + if (!channels || !channels[channelId]) { + return state; + } + + const nextChannels = {...channels}; + Reflect.deleteProperty(nextChannels, channelId); + return { + ...state, + [CHANNEL_RESOURCE_TYPE]: nextChannels, + }; + } + case RenderPermissionTypes.CLEAR_RENDER_DECISIONS: + case UserTypes.LOGOUT_SUCCESS: + return {}; + default: + return state; + } +} + +// Composed by hand rather than with combineReducers, which gives a child no access to its +// siblings. Mirrors how posts.ts threads state.posts into postsInChannel. +export default function renderPermissions(state: Partial = {}, action: MMReduxAction): RenderPermissionsState { + const nextInvalidatedAt = invalidatedAt(state.invalidatedAt, action); + const nextInvalidatedAtByResource = invalidatedAtByResource(state.invalidatedAtByResource, action); + + const nextState = { + byResource: byResource(state.byResource, action, nextInvalidatedAt, nextInvalidatedAtByResource), + invalidatedAt: nextInvalidatedAt, + invalidatedAtByResource: nextInvalidatedAtByResource, + }; + + if (state.byResource === nextState.byResource && + state.invalidatedAt === nextState.invalidatedAt && + state.invalidatedAtByResource === nextState.invalidatedAtByResource) { + // None of the children changed, so don't let the parent object change either. + return state as RenderPermissionsState; + } + + return nextState; +} diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/render_permissions.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/render_permissions.ts new file mode 100644 index 000000000000..f2aec634c8c5 --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/render_permissions.ts @@ -0,0 +1,12 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {RenderDecisionIdentifier, RenderPermissionEntry} from '@mattermost/types/render_permissions'; +import type {GlobalState} from '@mattermost/types/store'; + +// getRenderDecision returns the cached render-time decision for a single +// resource/action, or undefined if none has been fetched yet. +export function getRenderDecision(state: GlobalState, identifier: RenderDecisionIdentifier): RenderPermissionEntry | undefined { + const {resourceType, resourceId, action} = identifier; + return state.entities.renderPermissions.byResource[resourceType]?.[resourceId]?.[action]; +} diff --git a/webapp/channels/src/packages/mattermost-redux/src/store/initial_state.ts b/webapp/channels/src/packages/mattermost-redux/src/store/initial_state.ts index 6e0fe9ed9248..ad903363378a 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/store/initial_state.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/store/initial_state.ts @@ -249,6 +249,11 @@ const state: GlobalState = { values: {byTargetId: {}, byFieldId: {}}, groups: {byId: {}, byName: {}}, }, + renderPermissions: { + byResource: {}, + invalidatedAt: 0, + invalidatedAtByResource: {}, + }, }, errors: [], requests: { diff --git a/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.test.ts b/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.test.ts index 419ea04ce6c6..90b7e3beb9ec 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.test.ts @@ -686,5 +686,43 @@ describe('PostUtils', () => { delete (storedPostSansMetadata as any).metadata; expect(shouldUpdatePost(post, storedPostSansMetadata)).toBe(true); }); + + it('should return false when the received post carries no metadata to compare', () => { + const stored = TestHelper.getPostMock({ + ...storedPost, + metadata: {redacted_file_count: 1} as Post['metadata'], + }); + const post = {...storedPost}; + delete (post as any).metadata; + + expect(shouldUpdatePost(post, stored)).toBe(false); + }); + + // A policy change strips or restores file metadata without touching update_at. + it('should return true for same posts whose files became redacted', () => { + const stored = TestHelper.getPostMock({ + ...storedPost, + metadata: {files: [{id: 'file1'}]} as unknown as Post['metadata'], + }); + const post = TestHelper.getPostMock({ + ...storedPost, + metadata: {redacted_file_count: 1} as Post['metadata'], + }); + + expect(shouldUpdatePost(post, stored)).toBe(true); + }); + + it('should return true for same posts whose files stopped being redacted', () => { + const stored = TestHelper.getPostMock({ + ...storedPost, + metadata: {redacted_file_count: 1} as Post['metadata'], + }); + const post = TestHelper.getPostMock({ + ...storedPost, + metadata: {files: [{id: 'file1'}]} as unknown as Post['metadata'], + }); + + expect(shouldUpdatePost(post, stored)).toBe(true); + }); }); }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.ts b/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.ts index 4cea8d1f99de..65f012a5f2b8 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/utils/post_utils.ts @@ -250,6 +250,15 @@ export function shouldUpdatePost(receivedPost: Post, storedPost?: Post): boolean return true; } + // A policy change strips or restores file metadata without touching update_at. Both sides + // must carry metadata: a response that omits it entirely (some endpoints do) must not + // clobber what's stored, which is what the check above guards in the other direction. + if (storedPost.metadata && receivedPost.metadata && + (storedPost.metadata.redacted_file_count ?? 0) !== (receivedPost.metadata.redacted_file_count ?? 0) + ) { + return true; + } + // The stored post is the same as the one we've received return false; } diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 29b1ed19ba7e..d2ffb807bfea 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -122,6 +122,7 @@ import type {UserPropertyField, UserPropertyFieldPatch} from '@mattermost/types/ import type {Reaction} from '@mattermost/types/reactions'; import type {Recap, CreateRecapRequest, ScheduledRecap, ScheduledRecapInput, RecapLimitStatus} from '@mattermost/types/recaps'; import type {RemoteCluster, RemoteClusterAcceptInvite, RemoteClusterPatch, RemoteClusterWithPassword} from '@mattermost/types/remote_clusters'; +import type {ActionSearchRequest, ActionSearchResponse} from '@mattermost/types/render_permissions'; import type {UserReport, UserReportFilter, UserReportOptions} from '@mattermost/types/reports'; import type {Role} from '@mattermost/types/roles'; import type {SamlCertificateStatus, SamlMetadataResponse} from '@mattermost/types/saml'; @@ -5120,6 +5121,17 @@ export default class Client4 { ); }; + searchAccessControlDecisionActions = (resourceType: string, resourceId: string, actions?: string[]) => { + const body: ActionSearchRequest = {resource: {type: resourceType, id: resourceId}}; + if (actions && actions.length > 0) { + body.actions = actions; + } + return this.doFetch( + `${this.getBaseRoute()}/access_control/decisions/actions/search`, + {method: 'post', body: JSON.stringify(body)}, + ); + }; + searchChildAccessControlPolicyChannels = (policyId: string, term: string, opts: ChannelSearchOpts, teamId?: string) => { const teamParam = teamId ? `?team_id=${encodeURIComponent(teamId)}` : ''; return this.doFetch( diff --git a/webapp/platform/client/src/websocket_events.ts b/webapp/platform/client/src/websocket_events.ts index a30a71738681..093f099d007e 100644 --- a/webapp/platform/client/src/websocket_events.ts +++ b/webapp/platform/client/src/websocket_events.ts @@ -84,6 +84,7 @@ export const enum WebSocketEvents { ChannelBookmarkDeleted = 'channel_bookmark_deleted', ChannelBookmarkSorted = 'channel_bookmark_sorted', ChannelAccessControlUpdated = 'channel_access_control_updated', + PermissionPolicyUpdated = 'permission_policy_updated', TeamAccessControlUpdated = 'team_access_control_updated', PresenceIndicator = 'presence', PostedNotifyAck = 'posted_notify_ack', // This isn't currently used by the web app diff --git a/webapp/platform/client/src/websocket_message.ts b/webapp/platform/client/src/websocket_message.ts index 8f8c341b3c8a..cfd3736ba39f 100644 --- a/webapp/platform/client/src/websocket_message.ts +++ b/webapp/platform/client/src/websocket_message.ts @@ -48,6 +48,7 @@ export type WebSocketMessage = ( Messages.ChannelBookmarkSorted | Messages.ChannelAccessControlUpdated | + Messages.PermissionPolicyUpdated | Messages.TeamAccessControlUpdated | Messages.ChannelJoinRequestCreated | diff --git a/webapp/platform/client/src/websocket_messages.ts b/webapp/platform/client/src/websocket_messages.ts index 97a201cebc28..aa845d7ac4b8 100644 --- a/webapp/platform/client/src/websocket_messages.ts +++ b/webapp/platform/client/src/websocket_messages.ts @@ -269,6 +269,11 @@ export type ChannelAccessControlUpdated = BaseWebSocketMessage; }>; +// Emitted after a system-scoped (TypePermission) permission policy is created, updated, +// deleted, or toggled active. No payload — the policy is global so the client reconciles +// the currently visible channel only. +export type PermissionPolicyUpdated = BaseWebSocketMessage>; + export type TeamAccessControlUpdated = BaseWebSocketMessage; }>; diff --git a/webapp/platform/types/src/client4.ts b/webapp/platform/types/src/client4.ts index a12ae1849173..02179fb8e9c0 100644 --- a/webapp/platform/types/src/client4.ts +++ b/webapp/platform/types/src/client4.ts @@ -21,6 +21,7 @@ export type Options = { credentials?: 'omit' | 'same-origin' | 'include'; body?: any; signal?: RequestInit['signal']; + cache?: RequestInit['cache']; /** Per-request fetch cache mode, e.g. 'reload' to bypass a stale ETag for a single request */ ignoreStatus?: boolean; /** If true, status codes > 300 are ignored and don't cause an error */ duplex?: 'half'; /** Optional, but required for node clients. Must be 'half' for half-duplex fetch; 'full' is reserved for future use. See https://fetch.spec.whatwg.org/#dom-requestinit-duplex */ }; diff --git a/webapp/platform/types/src/render_permissions.ts b/webapp/platform/types/src/render_permissions.ts new file mode 100644 index 000000000000..a46011745cca --- /dev/null +++ b/webapp/platform/types/src/render_permissions.ts @@ -0,0 +1,77 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +// Decides whether to show a control, never whether to allow one: the server re-evaluates the PDP +// on every request, so a stale decision costs a wrong affordance and nothing more. +// +// "Render-time" means cached per resource, not evaluated live. Fetched lazily on a miss — a +// channel switch costs one request, revisiting it costs none — and never polled. Dropped only on +// the websocket events that change an input: channel_access_control_updated for one channel; +// permission_policy_updated, a current-user attribute or role change, or a config/license flip +// for the whole cache. +export type RenderPermissionDecision = { + allowed: boolean; + evaluated: boolean; + reason?: string; +}; + +export type ActionSearchResult = {action: {name: string}}; + +export type ActionSearchSubject = { + id: string; + type?: string; +}; + +export type ActionSearchPage = {next_token?: string}; + +export type ActionSearchRequest = { + resource: { + type: string; + id: string; + }; + actions?: string[]; // optional; omit for discovery mode + subject?: ActionSearchSubject; // reserved + page?: ActionSearchPage; // reserved +}; + +export type ActionSearchResponse = { + resource: { + type: string; + id: string; + }; + results: ActionSearchResult[]; + decisions: Record; + page?: ActionSearchPage; +}; + +export type RenderPermissionEntry = RenderPermissionDecision & { + generation: number; +}; + +// Also the batching key for the decision data loader. +export type RenderDecisionIdentifier = { + resourceType: string; + resourceId: string; + action: string; +}; + +// Client-only; always the current user's decisions. +export type RenderPermissionsState = { + byResource: { + [resourceType: string]: { + [resourceId: string]: { + [action: string]: RenderPermissionEntry; + }; + }; + }; + + // Completions at or below these predate the invalidation and are discarded, so an in-flight + // fetch can't repopulate a cache that was just cleared. Scoped separately from the cache-wide + // stamp so dropping one channel doesn't discard another channel's in-flight fetch. + invalidatedAt: number; + invalidatedAtByResource: { + [resourceType: string]: { + [resourceId: string]: number; + }; + }; +}; diff --git a/webapp/platform/types/src/store.ts b/webapp/platform/types/src/store.ts index b6971ca37b52..d0fec82e17f2 100644 --- a/webapp/platform/types/src/store.ts +++ b/webapp/platform/types/src/store.ts @@ -22,6 +22,7 @@ import type {PostsState} from './posts'; import type {PreferenceType} from './preferences'; import type {PropertiesState} from './properties'; import type {Recap, ScheduledRecap, RecapLimitStatus} from './recaps'; +import type {RenderPermissionsState} from './render_permissions'; import type { AdminRequestsStatuses, ChannelsRequestsStatuses, FilesRequestsStatuses, GeneralRequestsStatuses, @@ -105,6 +106,7 @@ export type GlobalState = { }; contentFlagging: ContentFlaggingState; properties: PropertiesState; + renderPermissions: RenderPermissionsState; }; errors: any[]; requests: {