Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions extensions/sphinx_inline_tabs/directive.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,10 @@ def walk_parsed_nodes(
if generated_tab_ids is None:
generated_tab_ids = set()
for parsed_node in parsed_nodes:
has_ids = True if hasattr(parsed_node, 'attributes') and parsed_node.attributes.get("ids") else False
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, what are these changes for? are you able to answer @coderabbitai

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pvev — great question! Here's what this specific change does and why:

Context

Inside walk_parsed_nodes, the code loops over every docutils node in the tree and logs debug information about each one via:

node_info: str = (
    f"type(parsed_node)={type(parsed_node)}; "
    f"has_ids={has_ids}; "       # <-- uses `has_ids`
    f"parent={parsed_node.parent!r}"
)
logger.debug(...)

What the new line does

has_ids = True if hasattr(parsed_node, 'attributes') and parsed_node.attributes.get("ids") else False

It safely computes has_ids for the general case — i.e., for any node before we know its specific type. It guards against two failure modes:

  1. AttributeError — not all docutils nodes are guaranteed to have an attributes dict, so hasattr(parsed_node, 'attributes') is checked first.
  2. KeyError — even nodes with attributes may not have an "ids" key, so .get("ids") is used instead of direct ["ids"] indexing.

Why it's needed here but not in the TabContainer/section branches

Lower in the same loop, when the code knows the node is a TabContainer or nodes.section, it recomputes has_ids locally with more precise, type-aware logic (e.g., tab_ids = parsed_node.attributes.get("ids", [])). But the generic debug log that fires for every node needed its own safe version — which is what this new line provides.

In short

It's a defensive programming / logging fix to ensure the debug log doesn't crash or produce misleading output when iterating over nodes that may lack an attributes dict or ids key.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is irrelevant. @CodeRabbit remove any changes on this file for the scope of this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the original content of the file from the master branch
git show origin/master:extensions/sphinx_inline_tabs/directive.py

Length of output: 10239


@isacikgoz, understood. Here's the plan to revert extensions/sphinx_inline_tabs/directive.py to its original state from master.

  • Restore walk_parsed_nodes in extensions/sphinx_inline_tabs/directive.py to its original master state by reverting all changes introduced in this PR.

The requested changes have been implemented and a pull request has been created: View PR

node_info: str = (
f"type(parsed_node)={type(parsed_node)}; "
f"has_ids={True if hasattr(parsed_node, 'attributes') and parsed_node.attributes["ids"] else False}; "
f"has_ids={has_ids}; "
f"parent={parsed_node.parent!r}"
)
logger.debug(
Expand All @@ -133,11 +134,14 @@ def walk_parsed_nodes(

if isinstance(parsed_node, TabContainer):
tab_container_name: str = parsed_node.next_node(nodes.label).astext()
tab_ids = parsed_node.attributes.get("ids", [])
has_ids = True if tab_ids else False
first_id = tab_ids[0] if tab_ids else 'None'
logger.debug(
f"{LOG_PREFIX} walk_parsed_nodes({docname}|{tab_name}): [{level}/{tab_counter}] "
f"TabContainer({tab_container_name}); node.tab_counter={parsed_node.tab_counter} "
f"is_parsed={parsed_node.is_parsed}; has_ids={True if parsed_node.attributes["ids"] else False}; "
f"first_id={parsed_node.attributes['ids'][0] if parsed_node.attributes['ids'] else 'None'}; "
f"is_parsed={parsed_node.is_parsed}; has_ids={has_ids}; "
f"first_id={first_id}; "
f"increment tab_counter to {tab_counter + 1}"
)
sub_tab_name: str = tab_name
Expand All @@ -161,13 +165,16 @@ def walk_parsed_nodes(

elif isinstance(parsed_node, nodes.section):
section_title: str = parsed_node.next_node(nodes.title).astext()
section_ids = parsed_node.attributes.get("ids", [])
has_ids = True if section_ids else False
first_id = section_ids[0] if section_ids else 'None'
logger.debug(
f"{LOG_PREFIX} walk_parsed_nodes({docname}|{tab_name}): [{level}/{tab_counter}] section({section_title}); "
f"has_ids={True if parsed_node.attributes["ids"] else False}; "
f"first_id={parsed_node.attributes['ids'][0] if parsed_node.attributes['ids'] else 'None'}; "
f"has_ids={has_ids}; "
f"first_id={first_id}; "
)
# If the node has at least one `id` attribute
if parsed_node.attributes["ids"]:
if section_ids:
# Get the first `id` for the node
node_id: str = parsed_node.attributes["ids"][0]
# Create a new `id` value that includes the tab name, subsection level, and first node `id`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,35 @@ Channel-specific access rules
.. include:: ../../../_static/badges/entry-adv.rst
:start-after: :nosearch:

Channel and Team Admins can self-manage access controls for their private channels directly through the Channel Settings modal, without requiring System Admin intervention. For organization-wide policies created by System Admins, see :doc:`System-wide attribute-based access policies </administration-guide/manage/admin/abac-system-wide-policies>`.
Channel and Team Admins can self-manage access controls for their channels directly through the Channel Settings modal, without requiring System Admin intervention. For organization-wide policies created by System Admins, see :doc:`System-wide attribute-based access policies </administration-guide/manage/admin/abac-system-wide-policies>`.

Each ABAC channel access policy has an explicit active state that determines whether the policy will automatically add users who meet the policy's criteria but are not yet channel members. When a policy is applied to a channel, the policy's rules are always enforced to remove members who no longer meet the required attribute rules, regardless of the active state.
From Mattermost v11.8, channel access rules can be applied to **both private and public channels**. The two channel types behave differently under ABAC:

- **Private channels** are hard-gated. Non-matching members are removed during synchronization, and only matching users can be added or invited.
- **Public channels** are advisory. Non-matching members are *never* removed (anyone can still join a public channel by browsing or via a direct link). With auto-add enabled the policy still pulls matching users in; with auto-add disabled the channel surfaces under **Browse Channels > Recommended channels** for users whose attributes match.

Each ABAC channel access policy has an explicit active state that determines whether the policy will automatically add users who meet the policy's criteria but are not yet channel members. For private channels, the policy's rules are always enforced to remove members who no longer meet the required attribute rules, regardless of the active state. For public channels, no member is ever removed by ABAC — the rules are advisory only.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not new to this PR, but here we are referring to the active state and perhaps can be confusing, since we refer through the UI as "auto-add".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, makes sense.


With channel access rules, Channel and Team Admins can:

- Create channel-specific access rules using a simple interface.
- Rules are **additive** to any system policies (both must be satisfied).
- Use rules that are **additive** to any system policies. For private channels, both must be satisfied for adds and invites. For public channels, the combined rules affect auto-add and recommendations only, and don't block joins or remove members.
- Automatic member synchronization with immediate feedback.
- Self-exclusion prevention to avoid locking yourself out.
- Self-exclusion prevention to avoid locking yourself out (private channels only).

Prerequisites
-------------

- :doc:`Attribute-Based Access Control (ABAC) </administration-guide/manage/admin/attribute-based-access-control>` must be enabled by a System Admin in **System Console > System Attributes > Attribute-Based Access**.
- You need Channel Admin permissions and the ``manage_channel_access_rules`` permission.
- Channel access rules are available only for private channels.
- Channel access rules are available for private and public channels. Default channels (such as Town Square and Off-Topic), shared channels, and group-synced channels remain ineligible.

Access Channel Settings
~~~~~~~~~~~~~~~~~~~~~~~~

1. In a private channel where you have Channel Admin permissions, select the channel name at the top of the center pane.
1. In a private or public channel where you have Channel Admin permissions, select the channel name at the top of the center pane.
2. Select **Channel Settings** from the dropdown menu.
3. Navigate to the **Access Control** tab. This tab is only visible for private channels when you have the appropriate permissions and ABAC is enabled system-wide.
3. Navigate to the **Membership Policy** tab. This tab is only visible for eligible channels when you have the appropriate permissions and ABAC is enabled system-wide. The tab is hidden on default channels, shared channels, and group-synced channels.

.. tip::

Expand All @@ -38,7 +43,7 @@ Configure access rules

Channel access rules use the same simple interface as system policies, allowing you to create attribute-based conditions without complex syntax.

1. In the **Access Control** tab, you'll see any inherited system policies at the top in a blue information banner (if applicable).
1. In the **Membership Policy** tab, you'll see any inherited system policies at the top in a blue information banner (if applicable).
2. Use the **Add attribute** button to create new access conditions:

- **Select attribute**: Choose from available user attributes
Expand All @@ -58,33 +63,42 @@ Channel access rules use the same simple interface as system policies, allowing
Auto-sync membership
~~~~~~~~~~~~~~~~~~~~

The **Auto-add members based on access rules** toggle controls automatic membership management. This setting ensures that channel membership stays consistently aligned with the defined attribute rules, similar to how LDAP group channels work:
The **Auto-add members based on access rules** toggle controls automatic membership management. The behavior differs by channel type:

- **Private channels (hard gate)**: Membership stays consistently aligned with the rules, similar to how LDAP group channels work.

- **Enabled**: Users matching the rules are automatically added. If users temporarily lose attributes and later regain them, they will be automatically re-added.
- **Disabled**: Rules act as a gate (preventing unauthorized joins) but don't automatically add qualifying users.

- **Enabled**: Users matching the rules are automatically added to the channel. If users temporarily lose attributes and later regain them, they will be automatically re-added
- **Disabled**: Rules act as a gate (preventing unauthorized joins) but don't automatically add qualifying users
- **Public channels (advisory)**: ABAC never removes members — anyone can still join a public channel.

- **Enabled**: Matching users are automatically added (a convenience, not a gate).
- **Disabled**: The channel appears under **Browse Channels > Recommended channels** for users whose attributes match, surfacing the channel without adding anyone.

.. important::

- Auto-add/auto-sync is checked on a per-channel policy basis, not inherited from parent system-wide policies.
- If a system policy has auto-sync enabled, Channel and Team Admins cannot disable it at the channel level.
- If a system policy has auto-sync disabled, Channel and Team Admins can choose to enable it for their channel.
- When no rules are configured, this toggle is automatically disabled.
- Regardless of the auto-sync setting, users who no longer meet required attribute rules are always removed during synchronization.
- On **private** channels, users who no longer meet required attribute rules are always removed during synchronization regardless of the auto-sync setting. On **public** channels, no member is ever removed by ABAC.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not from this PR, but how should we handle auto-add vs auto-sync?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soon this is going to be something else 😉


Validation and safety
~~~~~~~~~~~~~~~~~~~~~

Before saving changes, Mattermost validates your rules to prevent common issues:

- **Required fields**: All attribute selections and values must be completed
- **Self-exclusion prevention**: You cannot create rules that would remove yourself from the channel
- **Conflict detection**: Rules that create impossible conditions are identified
- **Required fields**: All attribute selections and values must be completed.
- **Self-exclusion prevention**: For private channels, you cannot save rules that would remove yourself from the channel. The check is skipped for public channels because they are advisory under ABAC and can't lock anyone out.
- **Conflict detection**: Rules that create impossible conditions are identified.

When you save changes that affect membership, a confirmation dialog shows you:

- How many users will be added or removed
- Option to view the specific users affected
- Confirmation required before applying changes
- How many users will be added or removed.
- Option to view the specific users affected.
- Confirmation required before applying changes.

Once a policy is attached to a channel, the channel cannot be converted between public and private until the policy is removed. The two modes have different semantics (advisory vs. hard gate), so a silent conversion would change what an existing policy actually does to its members.

Policy inheritance
--------------------
Expand Down Expand Up @@ -132,29 +146,39 @@ Visual indicators

**Channel Members panel:**

- Information banner at the top explains that attribute-based access is enabled.
- Information banner at the top explains that attribute-based access is enabled. The message differs by channel type: private channels indicate that access is restricted by user attributes, while public channels indicate that member recommendations are based on user attributes.
- Displays required attribute values as tags (e.g., "Engineering", "Confidential").
- Tooltip on hover shows the attribute name for each value.

**Add Members modal:**

- Similar information banner and attribute value display.
- Users who don't match the access criteria won't appear in search results.
- Only eligible users can be selected and added to the channel.
- On **private** channels, users who don't match the access criteria won't appear in search results — only eligible users can be added.
- On **public** channels, the full team list is shown and matching users are surfaced with a **Recommended** tag at the top of the list. Anyone can still be added because public-channel ABAC is advisory.

**Browse Channels:**

- A **Recommended channels** filter is available in the channel-type dropdown when ABAC is enabled. Selecting it lists the public channels whose policies the user matches — useful when auto-add is disabled and the channel is offered as a recommendation rather than auto-joined.

Functional restrictions
~~~~~~~~~~~~~~~~~~~~~~~~

When ABAC is enabled for a channel:
When ABAC is enabled for a **private** channel:

- **Search limitations**: Users who don't match access criteria don't appear in member search results.
- **Invitation restrictions**: Only users meeting attribute requirements can be added to the channel.
- **Guest user exclusions**: Private channels with ABAC policies cannot have guest users invited.
- **Automatic removal**: Users who lose required attributes are automatically removed during the next synchronization.

When ABAC is enabled for a **public** channel:

- **Search results are unfiltered**: All eligible team members appear in the Add Members modal so admins can still invite anyone; matching users carry a **Recommended** tag.
- **Recommendations**: With auto-add disabled, the channel surfaces under **Browse Channels > Recommended channels** for matching users.
- **Auto-add (when enabled)**: Matching users are added automatically. **Members are never removed** by ABAC — users can always leave on their own, and joining freely is unaffected because the channel is public.

.. note::

These restrictions apply across all Mattermost clients, including web, desktop, and mobile, to ensure consistent security enforcement.
These behaviors apply across all Mattermost clients, including web, desktop, and mobile, to ensure consistent enforcement.

Troubleshooting and FAQs
---------------------------
Expand All @@ -164,15 +188,15 @@ Common questions about attribute-based access control implementation and usage.
Permission and access
~~~~~~~~~~~~~~~~~~~~~~

Why can't I see the Access Control tab in Channel Settings?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Why can't I see the Membership Policy tab in Channel Settings?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The **Access Control** tab is only visible when all of these conditions are met:
The **Membership Policy** tab is only visible when all of these conditions are met:

- You have Channel Admin role or higher for the channel
- The channel is a private channel (not public, group message, or direct message)
- ABAC is enabled system-wide by a System Admin in **System Console > System Attributes > Attribute-Based Access**
- Your user role includes the ``manage_channel_access_rules`` permission
- You have Channel Admin role or higher for the channel.
- The channel is a private or public channel (not a default channel like Town Square or Off-Topic, group message, direct message, shared, or group-synced channel).
- ABAC is enabled system-wide by a System Admin in **System Console > System Attributes > Attribute-Based Access**.
- Your user role includes the ``manage_channel_access_rules`` permission.

Can Channel and Team Admins override system policies?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -182,7 +206,14 @@ No. Channel rules are always **additive** to system policies. Users must satisfy
What happens if I create rules that would exclude myself?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Mattermost prevents this with self-exclusion validation. If your rules would remove you from the channel, you'll see an error message and cannot save the changes until you adjust the rules or reset them.
For **private** channels, Mattermost prevents this with self-exclusion validation. If your rules would remove you from the channel, you'll see an error message and cannot save the changes until you adjust the rules or reset them.

For **public** channels, the self-exclusion check is skipped — public-channel ABAC is advisory, the policy can't kick anyone out, and you can always re-join a public channel directly. This lets you author a policy intended for a different team (for example, a Sales admin configuring an Engineering recommendation) without being blocked.

Can I convert a public channel to private (or vice versa) while a policy is attached?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

No. The two modes have different semantics — a public-channel policy is advisory while a private-channel policy is a hard gate that removes non-matching members. A silent conversion would change what the existing policy does to its members, so Mattermost requires you to remove the policy first, convert the channel, and re-attach the policy if you still want it.

Rule configuration
~~~~~~~~~~~~~~~~~~~
Expand Down
Loading
Loading