Enhance SAML configuration instructions for Mattermost#8914
Enhance SAML configuration instructions for Mattermost#8914
Conversation
Updated instructions for configuring SAML attributes and claims in Entra for Mattermost integration, including detailed explanations of required and additional claims.
|
Newest code from mattermost has been published to preview environment for Git SHA d340aa0 |
|
Newest code from mattermost has been published to preview environment for Git SHA 83d5fed |
|
Newest code from mattermost has been published to preview environment for Git SHA 201af0c |
|
Newest code from mattermost has been published to preview environment for Git SHA a09e76c |
Added an image reference for the Tenant ID location in Entra.
|
Newest code from mattermost has been published to preview environment for Git SHA 8010315 |
📝 WalkthroughWalkthroughDocumentation for Mattermost SSO with Entra ID was reorganized: the "Before you begin" include was replaced by an in-page section that adds X.509 certificate generation instructions and a Bash script; Entra role prerequisites and the enterprise app setup flow were rewritten with expanded claims-to-Mattermost mapping and updated step references. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Newest code from mattermost has been published to preview environment for Git SHA ee1196f |
|
Newest code from mattermost has been published to preview environment for Git SHA 40d6d50 |
There was a problem hiding this comment.
Pull request overview
Updates the Mattermost documentation for configuring SAML with Microsoft Entra ID, with expanded guidance on setting up Attributes & Claims and identifying the correct Entra IDs needed for Mattermost SAML settings.
Changes:
- Switches the Entra enterprise app setup steps to the Entra admin center “Non-gallery” flow.
- Adds a detailed mapping explanation and a sample claim mapping table for Mattermost SAML attributes.
- Adds screenshots for the Entra Attributes & Claims page and where to find the Tenant ID.
Reviewed changes
Copilot reviewed 1 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| source/administration-guide/onboard/sso-saml-entraid.rst | Rewrites Entra setup steps; adds claim mapping guidance, notes, and new screenshots. |
| source/images/entra-tenant-id.png | New screenshot illustrating where to find Tenant ID in Entra. |
| source/images/entra-attributes-and-claims.png | New screenshot illustrating recommended simplified claim names in Entra. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA d2b8e2b |
|
Newest code from mattermost has been published to preview environment for Git SHA 7f0d3f8 |
|
Newest code from mattermost has been published to preview environment for Git SHA 99c7757 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA 9c215f5 |
|
Newest code from mattermost has been published to preview environment for Git SHA 93e4446 |
|
Newest code from mattermost has been published to preview environment for Git SHA 64ba8a1 |
Added instructions for generating a self-signed X.509 certificate for SAML configuration, including a Bash script reference and environment variable details.
|
Newest code from mattermost has been published to preview environment for Git SHA 26cdcb7 |
Updated the Bash script reference for generating a self-signed Service Provider certificate, consolidating information and improving clarity.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
source/administration-guide/onboard/sso-saml-entraid.rst (1)
132-140: Add a fallback note for Entra UI navigation drift.Microsoft Entra UI labels and navigation paths change frequently. Add a short fallback instruction (for example, "If labels differ from the menu shown, use the search bar to locate Enterprise apps") to reduce friction for novice admins who may encounter outdated or inconsistent UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 132 - 140, Add a short fallback note after step 2 (the "In the left navigation menu, select **Entra ID > Enterprise applications**" instruction) to help when Microsoft Entra UI labels or navigation differ; for example, instruct editors to add a sentence like "If the menu labels differ, use the Entra search bar to find 'Enterprise applications' or 'Enterprise apps'." Ensure the note is also referenced where other menu paths are used (e.g., the "Manage > Users and groups" and "Manage > Single sign-on" steps) so readers are reminded to search the UI if they cannot find those exact labels.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/administration-guide/onboard/sso-saml-entraid.rst`:
- Line 117: Update the sentence that claims the script outputs
"mattermost-x509.key" and "mattermost-x509.crt" to clarify these are the default
names: note that CRT_FILENAME (and related variables) can be configured so
actual output filenames may differ; reference the gencert.sh script and mention
"by default" and/or the variable name CRT_FILENAME so readers know to check or
override the filename if they changed configuration.
- Around line 188-191: Replace the current note admonition with a warning
admonition so the SAML username-claim guidance is shown as a risk (change the
RST block that starts with ".. note::" and contains the lines referencing
"user.mailnickname", "user.userprincipalname" and "mailnickname" to use "..
warning::" instead); ensure the block content remains identical besides the
admonition type so the explanation about using mailnickname (and the fallback
option for custom Entra attribute or UPN transformation) is preserved under the
warning.
- Around line 113-115: The chmod line currently sets restrictive permissions on
$CERT (the public certificate) instead of the private key; change the permission
target from $CERT to $KEY and ensure the private key file ($KEY) is set to a
restrictive mode (e.g., chmod 600 $KEY or chmod 400 $KEY) so the private key is
properly protected while leaving the certificate permissions unchanged.
- Line 105: Replace the CA-style basicConstraints in the OpenSSL extfile
invocation so the SAML Service Provider certificate is an end-entity, not a CA:
locate the extfile invocation that builds subjectAltName (the line containing
extfile <(echo -e
"...basicConstraints=critical,CA:true,pathlen:0\nsubjectAltName=${CRT_SAN:-..."}"))
and change the basicConstraints to basicConstraints=critical,CA:false and remove
the pathlen entry; keep the subjectAltName/CRT_SAN handling unchanged.
---
Nitpick comments:
In `@source/administration-guide/onboard/sso-saml-entraid.rst`:
- Around line 132-140: Add a short fallback note after step 2 (the "In the left
navigation menu, select **Entra ID > Enterprise applications**" instruction) to
help when Microsoft Entra UI labels or navigation differ; for example, instruct
editors to add a sentence like "If the menu labels differ, use the Entra search
bar to find 'Enterprise applications' or 'Enterprise apps'." Ensure the note is
also referenced where other menu paths are used (e.g., the "Manage > Users and
groups" and "Manage > Single sign-on" steps) so readers are reminded to search
the UI if they cannot find those exact labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed1752f0-a712-4a34-a8d9-da4475b54733
⛔ Files ignored due to path filters (2)
source/images/entra-attributes-and-claims.pngis excluded by!**/*.pngsource/images/entra-tenant-id.pngis excluded by!**/*.png
📒 Files selected for processing (1)
source/administration-guide/onboard/sso-saml-entraid.rst
| .. note:: | ||
| Use ``user.mailnickname`` rather than ``user.userprincipalname`` as the source for the username claim. The user principal name is typically formatted as an email address (``user@domain.com``), but Mattermost usernames cannot contain the ``@`` character, so SAML logins using ``user.userprincipalname`` will fail. The mail nickname is the local part of the email address (the portion before ``@``) and maps cleanly to a valid Mattermost username. | ||
|
|
||
| 10. Select **Edit** in the **SAML Certificates** section. Select **Sign SAML response and assertion** for **Signing Option** and **SHA-256** for **Signing Algorithm** then select **Save**. | ||
| 11. Select the **Certificate (Base64)** Download link in the **SAML Certificates** section. This is the **Identity Provider Public Certificate** to be uploaded to Mattermost. | ||
| 12. In the **Mattermost SAML** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. The Import dialog says to upload a certificate with a file extension ``.cer``, but ``.crt`` files are also accepted. Upload the file then select **Add**. | ||
| 13. Select the ``...`` to the right of the imported certificate details, select **Activate token encryption certificate**, then select **Yes** to activate. | ||
| 14. On the **Home** page for **Microsoft Entra ID**, select the **Overview** link in the left navigation menu and copy the **Tenant ID** value. The **Tenant ID** will be used in Mattermost **SAML 2.0** URL settings. | ||
| 15. In the left navigation menu, select **Manage > Enterprise applications**. Select the **Mattermost SAML** application then copy the **Application ID**. The **Application ID** will be used in the **Identity Provider Metadata URL** setting in the Mattermost **SAML 2.0** settings. | ||
| If your organization doesn't populate ``mailnickname`` consistently, another option is a custom Entra attribute or a transformation that strips the domain from the UPN. |
There was a problem hiding this comment.
Use a warning admonition for the username-claim failure case.
This block describes a likely misconfiguration that causes SAML login failure. It should be .. warning:: rather than .. note::.
Suggested minimal diff
- .. note::
+ .. warning::As per coding guidelines: “Use warning admonition for meaningful risk such as broken behavior … or likely user/admin mistakes with negative consequences.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 188 -
191, Replace the current note admonition with a warning admonition so the SAML
username-claim guidance is shown as a risk (change the RST block that starts
with ".. note::" and contains the lines referencing "user.mailnickname",
"user.userprincipalname" and "mailnickname" to use ".. warning::" instead);
ensure the block content remains identical besides the admonition type so the
explanation about using mailnickname (and the fallback option for custom Entra
attribute or UPN transformation) is preserved under the warning.
|
Newest code from mattermost has been published to preview environment for Git SHA 35dc2c3 |
| .. note:: | ||
| Use ``user.mailnickname`` rather than ``user.userprincipalname`` as the source for the username claim. The user principal name is typically formatted as an email address (``user@domain.com``), but Mattermost usernames cannot contain the ``@`` character, so SAML logins using ``user.userprincipalname`` will fail. The mail nickname is the local part of the email address (the portion before ``@``) and maps cleanly to a valid Mattermost username. | ||
|
|
||
| 10. Select **Edit** in the **SAML Certificates** section. Select **Sign SAML response and assertion** for **Signing Option** and **SHA-256** for **Signing Algorithm** then select **Save**. | ||
| 11. Select the **Certificate (Base64)** Download link in the **SAML Certificates** section. This is the **Identity Provider Public Certificate** to be uploaded to Mattermost. | ||
| 12. In the **Mattermost SAML** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. The Import dialog says to upload a certificate with a file extension ``.cer``, but ``.crt`` files are also accepted. Upload the file then select **Add**. | ||
| 13. Select the ``...`` to the right of the imported certificate details, select **Activate token encryption certificate**, then select **Yes** to activate. | ||
| 14. On the **Home** page for **Microsoft Entra ID**, select the **Overview** link in the left navigation menu and copy the **Tenant ID** value. The **Tenant ID** will be used in Mattermost **SAML 2.0** URL settings. | ||
| 15. In the left navigation menu, select **Manage > Enterprise applications**. Select the **Mattermost SAML** application then copy the **Application ID**. The **Application ID** will be used in the **Identity Provider Metadata URL** setting in the Mattermost **SAML 2.0** settings. | ||
| If your organization doesn't populate ``mailnickname`` consistently, another option is a custom Entra attribute or a transformation that strips the domain from the UPN. |
There was a problem hiding this comment.
Novice Nate (Friction): The note explains that Mattermost usernames can’t contain “@”, but Mattermost usernames also have other constraints (must start with a letter, 3–22 lowercase chars, only . - _ as symbols). user.mailnickname can be empty/inconsistent, contain uppercase, or be too short/long, which can still cause SAML sign-in failures even without “@”. Suggest expanding this note to include the full username constraints and recommending a claim transformation to lowercase/validate length (or using a dedicated custom attribute) when mailnickname doesn’t conform.
|
|
||
| 12. Select **Edit** in the **SAML Certificates** section. Select **Sign SAML response and assertion** for **Signing Option** and **SHA-256** for **Signing Algorithm**, then select **Save**. | ||
| 13. Select the **Certificate (Base64)** Download link in the **SAML Certificates** section. This is the **Identity Provider Public Certificate** to be uploaded to Mattermost. | ||
| 14. In the **Mattermost** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. The Import dialog says to upload a certificate with a file extension ``.cer``, but ``.crt`` files are also accepted. Upload the file then select **Add**. |
There was a problem hiding this comment.
Veteran Vince (Friction): “The Import dialog says to upload a certificate with a file extension .cer, but .crt files are also accepted.” is a behavioral claim that may not hold in all Entra tenants/UX variants. To avoid a dead-end for admins, suggest wording this more defensively (e.g., “If Entra rejects .crt, rename/convert the file to .cer and try again”).
| 14. In the **Mattermost** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. The Import dialog says to upload a certificate with a file extension ``.cer``, but ``.crt`` files are also accepted. Upload the file then select **Add**. | |
| 14. In the **Mattermost** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. If Entra accepts the ``.crt`` file, upload it and select **Add**. If Entra rejects it because it expects a ``.cer`` file, rename or convert the certificate to ``.cer`` and try again. |
| .. include:: sso-saml-ldapsync.rst | ||
| :start-after: :nosearch: | ||
|
|
||
| .. include:: sso-saml-faq.rst | ||
| :start-after: :nosearch: No newline at end of file | ||
| :start-after: :nosearch: |
There was a problem hiding this comment.
Editor Evie (Blocker): The .. include:: option lines at the end of the file are indented with a literal tab (e.g., \t:start-after: :nosearch:). Tabs can render inconsistently and can break Sphinx/reStructuredText directive option parsing in some toolchains. Suggest replacing the tabs with spaces (matching the indentation used earlier in the file for include options).
|
@Combs7th can you please give this a first pass review? |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA f772802 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
source/administration-guide/onboard/sso-saml-entraid.rst (8)
117-119: Cross-reference the permission fix after instructing to keep the key secure.Line 117 instructs users to "Keep the
.keyfile secure," but this advice is undermined by the incorrectchmodat line 113 (already flagged in past review comments). Consider adding a brief note here that proper file permissions are essential, and cross-reference the script's permission-setting step once that's corrected.As per coding guidelines: "Ensure commands include enough context for a technically literate admin to run them safely."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 117 - 119, Add a brief note after the sentence about keeping the ``.key`` file secure that explicitly reminds admins to set restrictive file permissions and cross-reference the script's permission-setting step in the gencert.sh documentation (the permission-setting/chmod step in the generate-certificates/gencert reference). Mention the specific artifacts (``mattermost-x509.key`` and ``mattermost-x509.crt``) and point readers to the corrected permission step so they run the secure chmod when following the script.
157-157: Clarify the relationship between Name ID and Mattermost's Id Attribute setting.Line 157 mentions that "Mattermost account binding is controlled by the Id Attribute (SAML) setting if you configure it, or by email otherwise." This is technically accurate, but the sentence is dense and mixes three concepts (Name ID format, Id Attribute setting, email fallback) in a way that may confuse Novice Nate. Consider breaking this into two sentences: one explaining Name ID's role in SAML, and another explaining how Mattermost determines account binding.
Suggested restructuring
-Set the **Name identifier format** and **Source attribute** values as required for your environment. The Name ID is part of the SAML assertion, but Mattermost account binding is controlled by the **Id Attribute (SAML)** setting if you configure it, or by email otherwise. If you want immutable user binding in Mattermost, add a separate ``Id`` claim under **Additional claims** and set its **Value** (source attribute) to an immutable Entra attribute such as ``user.objectid``. +Set the **Name identifier format** and **Source attribute** values as required for your environment. The Name ID is part of every SAML assertion. Mattermost determines account binding using the **Id Attribute (SAML)** setting if configured, or by email address otherwise. If you want immutable user binding in Mattermost, add a separate ``Id`` claim under **Additional claims** and set its **Value** (source attribute) to an immutable Entra attribute such as ``user.objectid``.As per coding guidelines: "Ensure clear transitions between steps and topics; avoid gaps that jump between subjects without connection."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` at line 157, Rewrite the dense sentence into two clear sentences: first state the role of the Name ID in the SAML assertion (e.g., "The Name ID is a SAML assertion attribute used to convey a user identifier and its format/source are set via Name identifier format and Source attribute"), and then explain Mattermost's account-binding order separately (e.g., "Mattermost binds accounts using the Id Attribute (SAML) if configured, and falls back to email otherwise; to ensure immutable binding add an additional 'Id' claim with its Value set to an immutable Entra attribute such as user.objectid—user.userprincipalname is a common human-readable Name ID but may break if renamed"). Reference the existing terms Name ID, Id Attribute (SAML), Additional claims, Id claim, user.objectid, and user.userprincipalname when making the split so readers can locate and update those phrases.
46-50: Clarify that CN and SAN flexibility applies specifically to SAML encryption certificates.Lines 46-50 state that the CN and SAN "don't need to match your Mattermost hostname" for "this SAML encryption certificate." While this is technically correct (Entra ID uses the certificate solely for encrypting SAML assertions, not for TLS hostname validation), novice administrators may find this confusing if they're used to TLS certificate requirements. Consider adding a brief clarification that this flexibility exists because the certificate is used for encryption key exchange in SAML, not for HTTPS/TLS.
Optional clarification
Add a sentence after line 47:
Common Name value. You can use any descriptive value for this SAML encryption certificate; it doesn't need to match your Mattermost hostname, though you can use the hostname if you prefer. +This certificate is used for SAML assertion encryption, not TLS hostname validation, so matching the hostname is optional.As per coding guidelines: "When reviewing documentation, evaluate it through the lens of Novice Nate—a novice IT Administrator with 1-2 years of experience ... can run CLI commands but wants to understand them first."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 46 - 50, Clarify that the CN and SAN flexibility applies specifically to the SAML encryption certificate by adding a brief sentence after the second bullet (after the line mentioning "this SAML encryption certificate") explaining that Entra ID uses the certificate only to encrypt SAML assertions (key exchange), not for HTTPS/TLS hostname validation, so the CN and SAN do not need to match the Mattermost hostname; reference the terms CN, SAN, SAML encryption certificate, Entra ID, and Mattermost hostname when inserting the sentence.
203-203: Clarify the .cer vs .crt file extension compatibility upfront.Line 203 mentions in passing that "The Import dialog says to upload a certificate with a file extension
.cer, but.crtfiles are also accepted." This is helpful information, but it's buried mid-instruction. Consider moving this note immediately after the filename is first mentioned, or using a.. note::admonition to make it more visible, since Novice Nate might hesitate or restart when seeing the .cer requirement.Suggested restructuring
-14. In the **Mattermost** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. The Import dialog says to upload a certificate with a file extension ``.cer``, but ``.crt`` files are also accepted. Upload the file then select **Add**. +14. In the **Mattermost** enterprise application settings, select **Security > Token encryption**. Select **Import Certificate** to import the Service Provider certificate. If you used the Bash script referenced in the **Before you begin** section, this is the ``mattermost-x509.crt`` file. + + .. note:: + The Import dialog requests a ``.cer`` file, but ``.crt`` files are also accepted. Both extensions represent X.509 certificates. + + Upload the file then select **Add**.As per coding guidelines: "Use
noteadmonition for clarifications, exceptions, non-blocking caveats, or extra context that helps the reader."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` at line 203, Move the clarification about .cer vs .crt up so it immediately follows the first mention of the Service Provider certificate filename (mattermost-x509.crt) in source/administration-guide/onboard/sso-saml-entraid.rst; replace the inline parenthetical with a visible admonition using the reST note directive (.. note::) that explicitly states the Import dialog requests .cer but .crt files are accepted, so readers see it before attempting the upload. Ensure the note is placed directly after the sentence that names mattermost-x509.crt and retains the example of the Bash script reference in the "Before you begin" section.
78-83: Enhance error handling for private key generation failure.Lines 78-83 check for key generation failure but use
exitwithout a non-zero exit code. This makes it harder to detect failures in automated workflows. Additionally, for Novice Nate's benefit, the error message could mention what to check (e.g., OpenSSL installation, permissions).Suggested improvement
if [ $? -ne 0 ]; then - echo "Error generating key" - exit + echo "Error generating key. Ensure OpenSSL is installed and you have write permissions in this directory." + exit 1 fiApply similar pattern to other error checks at lines 92-95 and 107-110.
As per coding guidelines: "Include expected output or success checks after key steps to help readers verify progress" and "Define technical terms briefly inline on first use rather than assuming reader knowledge."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 78 - 83, The private key generation uses "openssl genrsa -out $KEY 4096" and currently calls plain "exit" on failure; change the error handling to call "exit 1" (non-zero) and expand the echo to include actionable checks (e.g., verify OpenSSL is installed, check file permissions and available disk space), and mirror this pattern for the other key-check blocks that use plain "exit" (the subsequent openssl checks). Also add a brief expected-success message after key creation (e.g., "Private key written to $KEY") and define "private key" or "OpenSSL" inline on first use for the novice reader.
207-209: Enhance the GUID disambiguation note with examples.Lines 207-209 warn about confusing three different GUIDs but don't show what they look like or where exactly each appears in the UI. For Novice Nate, adding a brief example format (e.g., "Tenant ID: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, found on the Overview page") would make this more concrete and easier to verify.
As per coding guidelines: "Include expected output or success checks after key steps to help readers verify progress."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 207 - 209, Update the existing disambiguation note about GUIDs (the Tenant ID, Application ID, and Object ID) to include concrete example GUID formats and brief UI locations for each so readers can verify: show example formats like "Tenant ID: 11111111-1111-1111-1111-111111111111 (found on the Azure Entra Overview page)", "Application ID: 22222222-2222-2222-2222-222222222222 (found on the App registrations or enterprise app Overview)", and "Object ID: 33333333-3333-3333-3333-333333333333 (seen on the enterprise application's Properties page)"; also add a short verification check sentence telling the user which GUIDs Mattermost uses (Tenant ID and Application ID) and a quick success check such as "Verify: the Tenant ID and Application ID match the values entered into Mattermost's SAML settings."
143-145: Use consistent placeholder formatting for Mattermost URLs.Lines 143-145 use
<your-mattermost-url>as a placeholder in literal URL values. While this works, consider whether this should be formatted as inline code or whether the placeholder should use a different convention (e.g.,{your-mattermost-url}orYOUR_MATTERMOST_URL) to make it more obvious that it's a template. Check if nearby documentation uses a consistent pattern for URL placeholders.As per coding guidelines: "Match nearby pages and local repo patterns before applying generic Markdown or English style rules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 143 - 145, The three literal URL placeholders in the SSO SAML checklist ("Identifier (Entity ID)", "Reply URL (Assertion Consumer Service URL)", and "Sign on URL") use ``<your-mattermost-url>`` inconsistently with surrounding docs; update these three placeholders to match the project’s established pattern (for example replace ``<your-mattermost-url>`` with a consistent token such as ``{your-mattermost-url}`` or ``YOUR_MATTERMOST_URL``) and render them as inline code in each of the three bullet points so they read consistently (Identifier (Entity ID), Reply URL (Assertion Consumer Service URL), Sign on URL); before committing, verify the chosen placeholder format matches nearby pages in the repo and adjust to that canonical style.
233-236: Group encryption-related settings together for clarity.Lines 233-236 introduce the encryption settings (Enable Encryption, Service Provider Private Key, Service Provider Public Certificate) but split them across steps 11-13. For Novice Nate, it would be clearer to introduce these as a group with a brief explanation that all three settings work together. Consider adding a brief note before step 11 explaining that the next three steps configure SAML assertion encryption.
As per coding guidelines: "Ensure clear transitions between steps and topics; avoid gaps that jump between subjects without connection."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/onboard/sso-saml-entraid.rst` around lines 233 - 236, Add a brief transitional note before step 11 explaining that the next three settings (Enable Encryption, Service Provider Private Key, Service Provider Public Certificate) configure SAML assertion encryption and must be configured together; then group steps 11–13 under that note (e.g., "Configure SAML assertion encryption:") and keep step 14 separate for signing options (Sign Request / Signature Algorithm) while referencing the recommended algorithm RSAwithSHA256 in that step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/administration-guide/onboard/sso-saml-entraid.rst`:
- Line 70: Change the umask from 007 to 077 so private key files are never
group-readable at creation (replace the line setting "umask 007" with "umask
077"), and ensure the subsequent chmod targets the generated private key
variable ($KEY) (i.e., use "chmod 600 $KEY"). Also add a warning admonition near
the section to call out the security risk if umask is not 077 so readers know
this is required for private key safety.
- Line 240: Update the paragraph that begins "The **Test single sign-on with
Mattermost SAML** tool..." to explicitly state that Entra ID's HTTP-Redirect
(GET) binding places the encoded SAMLRequest and any signature parameters
(Signature and SigAlg) into the URL query string, which can exceed Entra ID's
~4096-byte limit; explain that enabling "Sign Request" appends Signature and
SigAlg to the query string and therefore increases the risk of hitting
AADSTS90015, and add that using HTTP-POST binding (which embeds the SAML payload
and signature in the request body) avoids this query-string size constraint.
- Line 199: Update the UI wording in the sentence that lists configurable SAML
attributes: replace the phrase "Preferred Language" with the exact System
Console label "Preferred language attribute" so the sentence reads "... Guest,
Admin, Nickname, and Preferred language attribute ..." to match Mattermost's
official SAML 2.0 authentication configuration terminology.
---
Nitpick comments:
In `@source/administration-guide/onboard/sso-saml-entraid.rst`:
- Around line 117-119: Add a brief note after the sentence about keeping the
``.key`` file secure that explicitly reminds admins to set restrictive file
permissions and cross-reference the script's permission-setting step in the
gencert.sh documentation (the permission-setting/chmod step in the
generate-certificates/gencert reference). Mention the specific artifacts
(``mattermost-x509.key`` and ``mattermost-x509.crt``) and point readers to the
corrected permission step so they run the secure chmod when following the
script.
- Line 157: Rewrite the dense sentence into two clear sentences: first state the
role of the Name ID in the SAML assertion (e.g., "The Name ID is a SAML
assertion attribute used to convey a user identifier and its format/source are
set via Name identifier format and Source attribute"), and then explain
Mattermost's account-binding order separately (e.g., "Mattermost binds accounts
using the Id Attribute (SAML) if configured, and falls back to email otherwise;
to ensure immutable binding add an additional 'Id' claim with its Value set to
an immutable Entra attribute such as user.objectid—user.userprincipalname is a
common human-readable Name ID but may break if renamed"). Reference the existing
terms Name ID, Id Attribute (SAML), Additional claims, Id claim, user.objectid,
and user.userprincipalname when making the split so readers can locate and
update those phrases.
- Around line 46-50: Clarify that the CN and SAN flexibility applies
specifically to the SAML encryption certificate by adding a brief sentence after
the second bullet (after the line mentioning "this SAML encryption certificate")
explaining that Entra ID uses the certificate only to encrypt SAML assertions
(key exchange), not for HTTPS/TLS hostname validation, so the CN and SAN do not
need to match the Mattermost hostname; reference the terms CN, SAN, SAML
encryption certificate, Entra ID, and Mattermost hostname when inserting the
sentence.
- Line 203: Move the clarification about .cer vs .crt up so it immediately
follows the first mention of the Service Provider certificate filename
(mattermost-x509.crt) in
source/administration-guide/onboard/sso-saml-entraid.rst; replace the inline
parenthetical with a visible admonition using the reST note directive (..
note::) that explicitly states the Import dialog requests .cer but .crt files
are accepted, so readers see it before attempting the upload. Ensure the note is
placed directly after the sentence that names mattermost-x509.crt and retains
the example of the Bash script reference in the "Before you begin" section.
- Around line 78-83: The private key generation uses "openssl genrsa -out $KEY
4096" and currently calls plain "exit" on failure; change the error handling to
call "exit 1" (non-zero) and expand the echo to include actionable checks (e.g.,
verify OpenSSL is installed, check file permissions and available disk space),
and mirror this pattern for the other key-check blocks that use plain "exit"
(the subsequent openssl checks). Also add a brief expected-success message after
key creation (e.g., "Private key written to $KEY") and define "private key" or
"OpenSSL" inline on first use for the novice reader.
- Around line 207-209: Update the existing disambiguation note about GUIDs (the
Tenant ID, Application ID, and Object ID) to include concrete example GUID
formats and brief UI locations for each so readers can verify: show example
formats like "Tenant ID: 11111111-1111-1111-1111-111111111111 (found on the
Azure Entra Overview page)", "Application ID:
22222222-2222-2222-2222-222222222222 (found on the App registrations or
enterprise app Overview)", and "Object ID: 33333333-3333-3333-3333-333333333333
(seen on the enterprise application's Properties page)"; also add a short
verification check sentence telling the user which GUIDs Mattermost uses (Tenant
ID and Application ID) and a quick success check such as "Verify: the Tenant ID
and Application ID match the values entered into Mattermost's SAML settings."
- Around line 143-145: The three literal URL placeholders in the SSO SAML
checklist ("Identifier (Entity ID)", "Reply URL (Assertion Consumer Service
URL)", and "Sign on URL") use ``<your-mattermost-url>`` inconsistently with
surrounding docs; update these three placeholders to match the project’s
established pattern (for example replace ``<your-mattermost-url>`` with a
consistent token such as ``{your-mattermost-url}`` or ``YOUR_MATTERMOST_URL``)
and render them as inline code in each of the three bullet points so they read
consistently (Identifier (Entity ID), Reply URL (Assertion Consumer Service
URL), Sign on URL); before committing, verify the chosen placeholder format
matches nearby pages in the repo and adjust to that canonical style.
- Around line 233-236: Add a brief transitional note before step 11 explaining
that the next three settings (Enable Encryption, Service Provider Private Key,
Service Provider Public Certificate) configure SAML assertion encryption and
must be configured together; then group steps 11–13 under that note (e.g.,
"Configure SAML assertion encryption:") and keep step 14 separate for signing
options (Sign Request / Signature Algorithm) while referencing the recommended
algorithm RSAwithSHA256 in that step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 649ea5b9-b005-4d10-b846-cb44fdcbbc78
📒 Files selected for processing (1)
source/administration-guide/onboard/sso-saml-entraid.rst
|
|
||
| #!/bin/bash | ||
|
|
||
| umask 007 |
There was a problem hiding this comment.
Review umask setting for private key generation security.
Line 70 sets umask 007, which creates files readable by the owner and group (mode 660/770). For a private key file, this allows group members to read the key. While the subsequent chmod 600 (once corrected to target $KEY) would fix this, the umask creates a window where the key is initially group-readable. Consider using umask 077 to ensure the private key is never readable by anyone except the owner from the moment of creation.
Suggested fix
- umask 007
+ umask 077As per coding guidelines: "Flag commands, settings, prerequisites, or sequencing that are misleading, incomplete, or unlikely to work as written" and "Use warning admonition for meaningful risk such as ... security exposure."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| umask 007 | |
| umask 077 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/administration-guide/onboard/sso-saml-entraid.rst` at line 70, Change
the umask from 007 to 077 so private key files are never group-readable at
creation (replace the line setting "umask 007" with "umask 077"), and ensure the
subsequent chmod targets the generated private key variable ($KEY) (i.e., use
"chmod 600 $KEY"). Also add a warning admonition near the section to call out
the security risk if umask is not 077 so readers know this is required for
private key safety.
| :alt: Entra Attributes & Claims page showing simplified claim names | ||
| :width: 100% | ||
|
|
||
| The **Guest**, **Admin**, **Nickname**, and **Preferred Language** attributes are also configurable in Mattermost if you want to drive them from Entra. Use the same pattern: pick a short claim name in Entra, set its source attribute, and enter the matching claim name in the corresponding Mattermost attribute field. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for SAML attribute configuration documentation
rg -n -C5 'Guest.*Attribute|Admin.*Attribute|Nickname.*Attribute|Preferred.*Language.*Attribute' source/ --type rst -g '!onboard/sso-saml-entraid.rst'Repository: mattermost/docs
Length of output: 35455
🏁 Script executed:
# Search for Locale attribute in SAML configuration
rg -n 'locale.*attribute|Locale.*Attribute' source/administration-guide/configure/authentication-configuration-settings.rst -i -A 5 -B 2Repository: mattermost/docs
Length of output: 1808
🏁 Script executed:
# Check the exact context in the file being reviewed
cat -n source/administration-guide/onboard/sso-saml-entraid.rst | sed -n '190,210p'Repository: mattermost/docs
Length of output: 2613
Update terminology to match official Mattermost SAML documentation.
Line 199 mentions "Preferred Language attributes" but the official System Console setting is labeled "Preferred language attribute" in the SAML 2.0 authentication configuration. All four attributes (Guest, Admin, Nickname, and Preferred language) are valid and configurable in Mattermost, so the guidance is sound—update the capitalization to "Preferred language attribute" for consistency with the official UI label.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/administration-guide/onboard/sso-saml-entraid.rst` at line 199, Update
the UI wording in the sentence that lists configurable SAML attributes: replace
the phrase "Preferred Language" with the exact System Console label "Preferred
language attribute" so the sentence reads "... Guest, Admin, Nickname, and
Preferred language attribute ..." to match Mattermost's official SAML 2.0
authentication configuration terminology.
|
|
||
| .. note:: | ||
|
|
||
| The **Test single sign-on with Mattermost SAML** tool in Microsoft Entra ID does not sign the request even if **Sign Request** is set to **True** in Mattermost. Depending on your security settings and key length, the Entra ID testing tool may successfully sign in while an actual sign in request from your Mattermost login page results in the error **AADSTS90015: Requested query string is too long.** since Entra ID handles the initial request with an HTTP GET redirect rather than HTTP POST. |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What causes Microsoft Entra ID SAML error AADSTS90015 "Requested query string is too long" and how does request signing affect this?
💡 Result:
The Microsoft Entra ID SAML error AADSTS90015, "Requested query string is too long" (also described officially as "QueryStringTooLong - The query string is too long"), occurs when the incoming authentication request's query string exceeds the 4096-byte limit enforced by Entra ID's HTTP listener input validation. Primary causes in SAML contexts: - Use of HTTP-Redirect binding (GET) for AuthnRequest, where the base64-encoded, deflated SAMLRequest parameter (plus RelayState) becomes oversized. Common when the SAML assertion includes many groups/claims or long parameters. - Signed SAML requests over HTTP-Redirect: Signing adds extra query parameters (Signature and SigAlg), substantially increasing length since the signature is appended to the URL rather than embedded in the body. - Long RelayState values (SAML standard limits to 80 bytes, but apps often exceed this). - Accumulated browser cookies or other query parameters pushing over the limit. Request signing exacerbates this specifically in HTTP-Redirect binding. When the service provider (SP) signs the AuthnRequest: - The signature is not embedded in the SAML XML (as in POST); instead, separate Signature={base64 sig} and SigAlg={algorithm} parameters are added to the query string. - This can add hundreds of bytes, easily exceeding limits on already large requests. Solutions: - Switch to HTTP-POST binding for the AuthnRequest (preferred for signed requests), which sends the SAMLRequest in the POST body (no query string limit issue). Configure in the SP application settings (e.g., vendor UI, web.config). - Disable SAML request signing if not required (Entra ID does not enforce or validate signed incoming AuthnRequests by default). - Shorten RelayState (ask app vendor). - Clear excessive browser cookies for microsoftonline.com domains as a temporary workaround. - Use tools like Fiddler to capture and measure the SAMLRequest query string size. Official Entra ID error reference confirms the description with no additional SAML-specific details. Microsoft Q&A threads from Entra staff provide the most direct SAML guidance.
Citations:
- 1: https://learn.microsoft.com/en-us/answers/questions/2260686/aadsts90015-requested-query-string-is-too-long
- 2: https://learn.microsoft.com/en-us/answers/questions/31352/how-to-fix-azure-single-sign-on-error-aadsts90015
- 3: https://learn.microsoft.com/en-us/answers/questions/1043724/okta-with-azure-sso-error-requested-query-string-t
- 4: https://learn.microsoft.com/en-us/entra/identity-platform/reference-error-codes
- 5: https://aka.ms/AADsigninsErrorCodes
Clarify the AADSTS90015 error mechanism and the role of request signing.
The explanation that Entra ID uses HTTP-Redirect (GET) is correct, but it omits critical context: the error occurs because GET binding places the SAML request and the signature parameters in the query string, which must not exceed 4096 bytes. When you enable signed requests, the Signature and SigAlg parameters are appended to the URL rather than embedded in the body, substantially increasing the query string size. Rewrite to explain that signing amplifies the risk of exceeding Entra ID's query string size limit, and note that switching to HTTP-POST binding avoids this constraint entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/administration-guide/onboard/sso-saml-entraid.rst` at line 240, Update
the paragraph that begins "The **Test single sign-on with Mattermost SAML**
tool..." to explicitly state that Entra ID's HTTP-Redirect (GET) binding places
the encoded SAMLRequest and any signature parameters (Signature and SigAlg) into
the URL query string, which can exceed Entra ID's ~4096-byte limit; explain that
enabling "Sign Request" appends Signature and SigAlg to the query string and
therefore increases the risk of hitting AADSTS90015, and add that using
HTTP-POST binding (which embeds the SAML payload and signature in the request
body) avoids this query-string size constraint.
Updated instructions for configuring SAML attributes and claims in Entra for Mattermost integration, including detailed explanations of required and additional claims.