Conversation
Contributor
Author
This was referenced May 5, 2026
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated Microsoft Entra (Azure AD) setup docs: added framed screenshots, made role-mapping required, expanded API-permission guidance (delegated vs application), revised manifest and token-claim instructions (accessTokenAcceptedVersion, optionalClaims, groupMembershipClaims), added client-secret/assignment warnings, and expanded troubleshooting. (≤50 words) ChangesEntra ID Setup Guide (single cohesive update)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
Comment |
impoiler
changed the title
Contributor
Confidence Score: 3/5Not ready to merge — prior-round issues around the overly-broad AppRoleAssignment permission and the Step 6/Step 8 group-claim conflict are still open in the file. The docs/enterprise/setting-up-entra.mdx — all five AppRoleAssignment.ReadWrite.All references and the Step 6 group-type selector guidance need attention. Important Files Changed
Reviews (6): Last reviewed commit: "docs: Setting up Microsoft Entra docs up..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/enterprise/setting-up-entra.mdx (1)
287-325: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftAdd the required Mintlify tabs for this configuration section.
This setup section should be presented using Web UI / API / config.json tabs, and the
config.jsonexample should align withtransports/config.schema.json.As per coding guidelines
docs/**/*.mdx: "Mintlify MDX documentation must have Web UI / API / config.json tabs; validate config.json examples against transports/config.schema.json".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 287 - 325, The Entra setup section is missing Mintlify tabs; wrap the configuration content in Web UI / API / config.json tabs and add a config.json tab example that matches the schema in transports/config.schema.json, including the documented fields (tenantId, clientId, clientSecret, audience, attributeRoleMappings, attributeTeamMappings, attributeBusinessUnitMappings and App ID URI where applicable). Ensure the config.json snippet uses the same keys and types as transports/config.schema.json, validate the example against that schema, and keep the Web UI and API tabs showing the UI steps and API notes respectively.
🧹 Nitpick comments (1)
docs/enterprise/setting-up-entra.mdx (1)
243-247: ⚡ Quick winClarify manifest schema variant for token-version setting.
Line 246 uses the legacy property name
"accessTokenAcceptedVersion": 2. In modern Microsoft Graph-format manifests (the current schema), this setting is represented as"api": { "requestedAccessTokenVersion": 2 }on theapinode. Both forms control the same underlying setting, but they target different manifest versions. Add a note specifying which manifest format this guide targets, or include both forms to prevent confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 243 - 247, The doc uses the legacy manifest property "accessTokenAcceptedVersion": 2 which can be confused with the modern Graph manifest format; update the text to clarify the manifest schema being targeted and show both variants: the legacy top-level "accessTokenAcceptedVersion": 2 and the modern "api": { "requestedAccessTokenVersion": 2 }, or explicitly state “this guide targets X manifest version” before the example so readers know which form to use (referencing accessTokenAcceptedVersion and api.requestedAccessTokenVersion).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 58-60: Fix the grammar inside the Note block that currently reads
"This step is optional. You can create custom roles if thats the preferred way.
Or you can map any attribute to role/team/business unit. Role mapping is
required step." — replace "thats" with "that's" and change "Role mapping is
required step." to "Role mapping is a required step." so the Note reads
correctly.
- Around line 164-176: Update the permission type for User.Read in the Entra
setup docs: change the two occurrences of `User.Read` currently listed under
Application permissions (in the bullet list and the Warning paragraph) to be
Delegated-only so they match Microsoft Graph's definition; ensure the bullet
list now only contains Application permissions (`User.Read.All`,
`GroupMember.Read.All`, `Group.Read.All`, `Application.Read.All`,
`AppRoleAssignment.ReadWrite.All`) and adjust the Warning text so it correctly
states that `openid`, `profile`, `email`, `offline_access`, and `User.Read` must
be Delegated while the remaining listed permissions are Application.
---
Outside diff comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 287-325: The Entra setup section is missing Mintlify tabs; wrap
the configuration content in Web UI / API / config.json tabs and add a
config.json tab example that matches the schema in
transports/config.schema.json, including the documented fields (tenantId,
clientId, clientSecret, audience, attributeRoleMappings, attributeTeamMappings,
attributeBusinessUnitMappings and App ID URI where applicable). Ensure the
config.json snippet uses the same keys and types as
transports/config.schema.json, validate the example against that schema, and
keep the Web UI and API tabs showing the UI steps and API notes respectively.
---
Nitpick comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 243-247: The doc uses the legacy manifest property
"accessTokenAcceptedVersion": 2 which can be confused with the modern Graph
manifest format; update the text to clarify the manifest schema being targeted
and show both variants: the legacy top-level "accessTokenAcceptedVersion": 2 and
the modern "api": { "requestedAccessTokenVersion": 2 }, or explicitly state
“this guide targets X manifest version” before the example so readers know which
form to use (referencing accessTokenAcceptedVersion and
api.requestedAccessTokenVersion).
🪄 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: a48f4498-018d-46eb-ab10-dead704441e2
📒 Files selected for processing (1)
docs/enterprise/setting-up-entra.mdx
impoiler
force-pushed
the
05-05-docs_setting_up_microsoft_entra_docs_updates
branch
from
b3a9b57 to
6ceaaa7
Compare
impoiler
force-pushed
the
05-05-fix_mcp_tool_group_rbac_check
branch
from
b1bee80 to
443e54f
Compare
impoiler
force-pushed
the
05-05-docs_setting_up_microsoft_entra_docs_updates
branch
from
6ceaaa7 to
1aaec7f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/enterprise/setting-up-entra.mdx (2)
331-337:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the leftover Okta wording and stale step references.
This Entra guide still says “Okta claims,” and it points
groupsto Step 5 /roleto Step 4 even though those steps now cover API permissions and client secrets. The mapping intro should reference the Entra token-claim setup consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 331 - 337, The doc still mentions "Okta claims" and stale step numbers; update the wording to consistently reference Entra token claims and remove/replace Step 4/Step 5 references. Specifically, in the paragraph describing attribute mappings (attributeRoleMappings, attributeTeamMappings, attributeBusinessUnitMappings) change "Okta claims" to "Entra token claims" and replace the references to `groups`/`role` pointing to Step 5/Step 4 with a neutral reference to the Entra token-claim setup (e.g., "the `groups` claim or a custom `role` claim configured in your Entra token-claim setup") so the text no longer points to outdated steps.
287-325: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd the required API/config tabs here, and include
appIdUriin the non-UI representation.This section only documents the Web UI flow, and the reference table below omits the
App ID URIfield introduced on Line 307. That leaves API/config users without a complete way to express the same setup.As per coding guidelines,
docs/**/*.mdx: Mintlify MDX documentation must have Web UI / API / config.json tabs; validate config.json examples against transports/config.schema.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 287 - 325, Add the missing API/config tabs and include the App ID URI field in the non-UI/config representation: update the UI docs section to provide Web UI / API / config.json tabs as required, add the `appIdUri` (key name `appIdUri`) to the Configuration Reference table alongside `tenantId`, `clientId`, `clientSecret`, and `audience`, and ensure any example config.json snippets in this file validate against transports/config.schema.json (adjust example values and schema fields to include `appIdUri` and confirm required/optional flags).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 421-422: Update the sentence that currently states "groups (which
is already in the token)" to clarify that the groups claim is not always present
and only available if the Entra/Azure AD tenant has the groups claim enabled in
the token configuration/manifest; specifically mention that switching Bifrost
mappings from using the roles attribute to the groups attribute requires
enabling the groups claim in the token configuration/manifest (see the earlier
token configuration/manifest steps) and link or reference the Attribute Mappings
section for details.
---
Outside diff comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 331-337: The doc still mentions "Okta claims" and stale step
numbers; update the wording to consistently reference Entra token claims and
remove/replace Step 4/Step 5 references. Specifically, in the paragraph
describing attribute mappings (attributeRoleMappings, attributeTeamMappings,
attributeBusinessUnitMappings) change "Okta claims" to "Entra token claims" and
replace the references to `groups`/`role` pointing to Step 5/Step 4 with a
neutral reference to the Entra token-claim setup (e.g., "the `groups` claim or a
custom `role` claim configured in your Entra token-claim setup") so the text no
longer points to outdated steps.
- Around line 287-325: Add the missing API/config tabs and include the App ID
URI field in the non-UI/config representation: update the UI docs section to
provide Web UI / API / config.json tabs as required, add the `appIdUri` (key
name `appIdUri`) to the Configuration Reference table alongside `tenantId`,
`clientId`, `clientSecret`, and `audience`, and ensure any example config.json
snippets in this file validate against transports/config.schema.json (adjust
example values and schema fields to include `appIdUri` and confirm
required/optional flags).
🪄 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: 011f5bbd-5ee1-4002-8c63-45ae4e6272f6
📒 Files selected for processing (1)
docs/enterprise/setting-up-entra.mdx
impoiler
force-pushed
the
05-05-fix_mcp_tool_group_rbac_check
branch
from
443e54f to
fbb9928
Compare
impoiler
force-pushed
the
05-05-docs_setting_up_microsoft_entra_docs_updates
branch
from
1aaec7f to
328686d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/enterprise/setting-up-entra.mdx (2)
331-337:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the stale Okta reference and step drift in Attribute Mappings.
This paragraph still says “Okta claims,” and the next sentence points readers to
groupsfrom Step 5 androlefrom Step 4 even though this guide configures groups in Step 6 and app roles in Steps 2/7. The section reads like a copy/paste from another provider and is now internally inconsistent.🛠️ Suggested doc fix
-Attribute mappings let you translate Entra claim values into Bifrost roles, teams, or business units without restructuring your Okta claims. Bifrost supports three mapping types: +Attribute mappings let you translate Entra claim values into Bifrost roles, teams, or business units without restructuring your Entra claims. Bifrost supports three mapping types: @@ -These mappings work with any Entra claim - the `groups` claim from Step 5, the custom `role` claim from Step 4, or any other claim your authorization server includes in the token (e.g., `department`, `organization`). +These mappings work with any Entra claim - the `groups` claim from Step 6, the `roles` claim from app-role assignments in Steps 2 and 7, or any other claim your authorization server includes in the token (e.g., `department`, `organization`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 331 - 337, Update the paragraph to remove the stale "Okta" reference and correct the step pointers: change "Okta claims" to "Entra claims", and update the example claim references so they point to where they are actually configured in this guide (the `groups` claim configured in Step 6 and the app `role` claim configured in Steps 2/7). Ensure the three mapping types (`attributeRoleMappings`, `attributeTeamMappings`, `attributeBusinessUnitMappings`) remain as-is and the sentence reads that these mappings work with any Entra claim (e.g., `groups` from Step 6, `role` from Steps 2/7, or other claims like `department` or `organization`).
287-325: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd the required Web UI / API /
config.jsontabs to the configuration section.This section only documents the Web UI path plus a reference table. The MDX guideline for this repo requires configuration docs to provide Web UI / API /
config.jsontabs, so readers currently don't get the API or file-based setup path here.🧭 Suggested structure
<Tabs> <Tab title="Web UI"> ...current Step 9 UI flow... </Tab> <Tab title="API"> ...equivalent API flow... </Tab> <Tab title="config.json"> ...schema-valid config example... </Tab> </Tabs>As per coding guidelines
docs/**/*.mdx: Mintlify MDX documentation must have Web UI / API / config.json tabs; validate config.json examples against transports/config.schema.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 287 - 325, The configuration docs only show the Web UI; add the required three-tab structure using the <Tabs> component with <Tab title="Web UI"> (retain the existing Step 9 UI content), <Tab title="API"> (add the equivalent API call/HTTP request and response examples for configuring Microsoft Entra/SCIM), and <Tab title="config.json"> (include a schema-valid JSON example matching fields like tenantId, clientId, clientSecret, audience, attributeRoleMappings, etc. and ensure the example validates against transports/config.schema.json). Also mention the restart note inside the Web UI tab and keep the Configuration Reference table available to all tabs.
♻️ Duplicate comments (1)
docs/enterprise/setting-up-entra.mdx (1)
421-422:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon’t say
groupsis already in the token.This fallback still contradicts Step 6:
groupsis only available after enabling/configuring that claim, so readers who skipped the token-claims step will switch mappings and hit the same failure mode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 421 - 422, The sentence incorrectly asserts that the groups attribute is already present in the token; update the wording in the Bifrost mappings fallback to clarify that the groups claim must be enabled/configured (reference Step 6 / Attribute Mappings), and instruct readers to enable/configure the groups claim in their token before switching mappings to use groups to avoid the same failure mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 241-247: The doc currently references the deprecated manifest
field accessTokenAcceptedVersion; update the manifest examples and text to use
the Microsoft Graph manifest shape by replacing accessTokenAcceptedVersion with
an api object containing requestedAccessTokenVersion (i.e.
api.requestedAccessTokenVersion: 2), and ensure any sample JSON, explanatory
text, and headings mention the new api.requestedAccessTokenVersion property
instead of accessTokenAcceptedVersion.
- Around line 165-188: Update the docs to replace the incorrect permission
string "AppRoleAssignment.Read.All" with the correct application permission
"AppRoleAssignment.ReadWrite.All" wherever it appears (the bullet list entry,
the Warning block, and the Note block) and ensure the text still states this
permission must be of type Application; no other changes to surrounding
explanatory text are needed.
---
Outside diff comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 331-337: Update the paragraph to remove the stale "Okta" reference
and correct the step pointers: change "Okta claims" to "Entra claims", and
update the example claim references so they point to where they are actually
configured in this guide (the `groups` claim configured in Step 6 and the app
`role` claim configured in Steps 2/7). Ensure the three mapping types
(`attributeRoleMappings`, `attributeTeamMappings`,
`attributeBusinessUnitMappings`) remain as-is and the sentence reads that these
mappings work with any Entra claim (e.g., `groups` from Step 6, `role` from
Steps 2/7, or other claims like `department` or `organization`).
- Around line 287-325: The configuration docs only show the Web UI; add the
required three-tab structure using the <Tabs> component with <Tab title="Web
UI"> (retain the existing Step 9 UI content), <Tab title="API"> (add the
equivalent API call/HTTP request and response examples for configuring Microsoft
Entra/SCIM), and <Tab title="config.json"> (include a schema-valid JSON example
matching fields like tenantId, clientId, clientSecret, audience,
attributeRoleMappings, etc. and ensure the example validates against
transports/config.schema.json). Also mention the restart note inside the Web UI
tab and keep the Configuration Reference table available to all tabs.
---
Duplicate comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 421-422: The sentence incorrectly asserts that the groups
attribute is already present in the token; update the wording in the Bifrost
mappings fallback to clarify that the groups claim must be enabled/configured
(reference Step 6 / Attribute Mappings), and instruct readers to
enable/configure the groups claim in their token before switching mappings to
use groups to avoid the same failure mode.
🪄 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: 4a164d1d-7d3f-4484-9813-6c35acdd2dbb
📒 Files selected for processing (1)
docs/enterprise/setting-up-entra.mdx
18 tasks
impoiler
force-pushed
the
05-05-docs_setting_up_microsoft_entra_docs_updates
branch
from
328686d to
1c02c4f
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/enterprise/setting-up-entra.mdx (3)
333-339:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClean up the stale Okta copy and broken step references here.
Line 333 still says “Okta claims”, and Line 339 points readers to Step 4 / Step 5 even though those sections are now client-secret and API-permission setup. That makes the mapping guidance actively misleading in the Entra guide.
✏️ Suggested wording
-Attribute mappings let you translate Entra claim values into Bifrost roles, teams, or business units without restructuring your Okta claims. Bifrost supports three mapping types: +Attribute mappings let you translate Entra claim values into Bifrost roles, teams, or business units without restructuring your token claims. Bifrost supports three mapping types: @@ -These mappings work with any Entra claim - the `groups` claim from Step 5, the custom `role` claim from Step 4, or any other claim your authorization server includes in the token (e.g., `department`, `organization`). +These mappings work with any Entra claim — for example the `roles` claim from your Entra app-role assignments, the `groups` claim enabled in the token-claims / manifest steps above, or any other claim included in the token (for example `department` or `organization`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 333 - 339, The paragraph currently refers to "Okta claims" and points to Step 4 / Step 5 which are no longer the claim-setup steps; update the wording to reference Entra instead of Okta and remove broken step links: in the sentence mentioning claims, replace "Okta claims" with "Entra claims" and change the Step 4 / Step 5 pointer to a neutral reference like "the groups claim configured earlier" or "the custom role claim you added in the Entra application" so readers are directed to the correct claim-configuration context; ensure the three mapping types (attributeRoleMappings, attributeTeamMappings, attributeBusinessUnitMappings) remain intact and that examples cite Entra claim names (e.g., groups, role, department) rather than Okta.
364-370:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the role-mapping evaluation rules internally consistent.
This section now describes three different behaviors for
attributeRoleMappings: first match wins, highest privilege wins, and a no-match Admin/Viewer fallback. Those rules are mutually incompatible, so admins will configure mappings differently depending on which sentence they trust.Please collapse this to one rule set and make the bullet + note use the same semantics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 364 - 370, The role-mapping text is inconsistent; update the section so all sentences use one consistent rule set for attributeRoleMappings: state that attributeRoleMappings are ordered and use first-match-wins (if multiple role mapping rules match the first matching rule is applied), keep Team and business unit mappings as “all matching rules apply,” and make the no-match fallback explicit (if no role mapping matches, the first user to sign in receives Admin and subsequent users receive Viewer). Ensure the bullets and the Note both reflect this single semantics and reference attributeRoleMappings and role mapping rules consistently.
289-327: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd the required Web UI / API /
config.jsontabs to this setup section.Step 9 currently documents only the UI path plus a reference table, so readers still don't get the standard API and copy-pastable
config.jsonflows this docs path requires. Please restructure this section into the required tabs and include a validatedconfig.jsonexample alongside the UI instructions.As per coding guidelines,
docs/**/*.mdx: Mintlify MDX documentation must have Web UI / API / config.json tabs; validate config.json examples against transports/config.schema.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 289 - 327, Update the "Step 9: Configure Bifrost" section to present three tabs: "Web UI", "API", and "config.json"; under the Web UI tab keep the existing UI step list and screenshot, under the API tab add curl/HTTP examples to create/update the SCIM provider (include request URL, headers, and JSON body fields matching tenantId, clientId, clientSecret, audience, attributeRoleMappings, attributeTeamMappings, attributeBusinessUnitMappings), and under the config.json tab provide a complete, copy-pastable JSON block named for the SCIM Entra provider that validates against transports/config.schema.json (include required keys tenantId, clientId, clientSecret and optional audience and mapping arrays), then ensure the docs text references the mapping behavior (first-match for attributeRoleMappings, all-match for attributeTeamMappings/business units) and add the restart warning after save as before.
♻️ Duplicate comments (1)
docs/enterprise/setting-up-entra.mdx (1)
423-424:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't imply the
groupsclaim is always present.“which is already in the token” conflicts with the earlier Step 6 / Step 8 guidance where
groupsis only available after enabling group claims. As written, this fallback can send readers into a broken setup.✏️ Suggested wording
-If you don't want to use app roles at all, switch your Bifrost mappings to use the `groups` attribute (which is already in the token) instead of `roles`. See [Attribute Mappings](`#attribute-mappings`). +If you don't want to use app roles at all, switch your Bifrost mappings to use the `groups` attribute instead of `roles` — but only after enabling group claims in the token configuration / manifest steps above. See [Attribute Mappings](`#attribute-mappings`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 423 - 424, The sentence implies the groups claim is always present; update the text so the fallback to using the `groups` attribute for Bifrost mappings is conditional — e.g., say "if group claims are enabled in Entra" — and remove or replace "which is already in the token" with a note referencing the earlier guidance (Step 6 / Step 8) that `groups` is only present after enabling group claims; keep mentions of `groups`, `roles`, `Bifrost mappings`, and the "Attribute Mappings" section so readers know where to configure it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 333-339: The paragraph currently refers to "Okta claims" and
points to Step 4 / Step 5 which are no longer the claim-setup steps; update the
wording to reference Entra instead of Okta and remove broken step links: in the
sentence mentioning claims, replace "Okta claims" with "Entra claims" and change
the Step 4 / Step 5 pointer to a neutral reference like "the groups claim
configured earlier" or "the custom role claim you added in the Entra
application" so readers are directed to the correct claim-configuration context;
ensure the three mapping types (attributeRoleMappings, attributeTeamMappings,
attributeBusinessUnitMappings) remain intact and that examples cite Entra claim
names (e.g., groups, role, department) rather than Okta.
- Around line 364-370: The role-mapping text is inconsistent; update the section
so all sentences use one consistent rule set for attributeRoleMappings: state
that attributeRoleMappings are ordered and use first-match-wins (if multiple
role mapping rules match the first matching rule is applied), keep Team and
business unit mappings as “all matching rules apply,” and make the no-match
fallback explicit (if no role mapping matches, the first user to sign in
receives Admin and subsequent users receive Viewer). Ensure the bullets and the
Note both reflect this single semantics and reference attributeRoleMappings and
role mapping rules consistently.
- Around line 289-327: Update the "Step 9: Configure Bifrost" section to present
three tabs: "Web UI", "API", and "config.json"; under the Web UI tab keep the
existing UI step list and screenshot, under the API tab add curl/HTTP examples
to create/update the SCIM provider (include request URL, headers, and JSON body
fields matching tenantId, clientId, clientSecret, audience,
attributeRoleMappings, attributeTeamMappings, attributeBusinessUnitMappings),
and under the config.json tab provide a complete, copy-pastable JSON block named
for the SCIM Entra provider that validates against transports/config.schema.json
(include required keys tenantId, clientId, clientSecret and optional audience
and mapping arrays), then ensure the docs text references the mapping behavior
(first-match for attributeRoleMappings, all-match for
attributeTeamMappings/business units) and add the restart warning after save as
before.
---
Duplicate comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 423-424: The sentence implies the groups claim is always present;
update the text so the fallback to using the `groups` attribute for Bifrost
mappings is conditional — e.g., say "if group claims are enabled in Entra" — and
remove or replace "which is already in the token" with a note referencing the
earlier guidance (Step 6 / Step 8) that `groups` is only present after enabling
group claims; keep mentions of `groups`, `roles`, `Bifrost mappings`, and the
"Attribute Mappings" section so readers know where to configure it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0855f6a-88f6-4674-a7f2-6c9ae860f7bd
📒 Files selected for processing (1)
docs/enterprise/setting-up-entra.mdx
18 tasks
impoiler
force-pushed
the
05-05-docs_setting_up_microsoft_entra_docs_updates
branch
from
1c02c4f to
75cbd06
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/enterprise/setting-up-entra.mdx (2)
337-337:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix incorrect step cross-references.
The text references incorrect steps:
- "the
groupsclaim from Step 5" → groups claim is configured in Step 6 (Token Claims), not Step 5 (API Permissions)- "the custom
roleclaim from Step 4" → app roles are created in Step 2 (Create App Roles), not Step 4 (Client Secret)📝 Suggested fix
-These mappings work with any Entra claim - the `groups` claim from Step 5, the custom `role` claim from Step 4, or any other claim your authorization server includes in the token (e.g., `department`, `organization`). +These mappings work with any Entra claim - the `groups` claim from Step 6, the custom `role` claim from Step 2, or any other claim your authorization server includes in the token (e.g., `department`, `organization`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` at line 337, Update the sentence that incorrectly references step numbers: change "the `groups` claim from Step 5" to "the `groups` claim from Step 6" and change "the custom `role` claim from Step 4" to "the custom `role` claim from Step 2" so the references to "groups claim" and "custom `role` claim" match the Token Claims (Step 6) and Create App Roles (Step 2) sections respectively.
362-368:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClarify contradictory role mapping behavior.
Line 362 states users are "not allowed to login" if no rule matches, but lines 367-368 state unmatched users receive a default role (Admin for first user, Viewer for subsequent). These statements contradict each other.
Please clarify when each behavior applies. For example, does line 362 apply only when
attributeRoleMappingsis configured, while lines 367-368 describe behavior when no mappings exist at all?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` around lines 362 - 368, Clarify the contradictory statements by specifying the two different cases: when attribute-based role mappings are configured (attributeRoleMappings / Role mappings list) and when no mappings exist at all; update the "Role mappings" bullet to say that if attributeRoleMappings is configured and no rule matches, users are not allowed to log in, and update the Note to state that when no attributeRoleMappings are configured at all (no mapping rules present), the first user to sign in is assigned Admin and subsequent users receive Viewer; reference the terms "Role mappings"/"attributeRoleMappings" and the Note about "first user Admin, subsequent Viewer" so readers can see which behavior applies in each scenario.
♻️ Duplicate comments (1)
docs/enterprise/setting-up-entra.mdx (1)
421-421:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify that
groupsrequires Step 6 configuration.The phrase "which is already in the token" implies the
groupsclaim is present by default, but Step 6 (Configure Token Claims) is marked optional. Users who skipped Step 6 won't have thegroupsclaim available.Consider adding a reference to Step 6:
📝 Suggested fix
-If you don't want to use app roles at all, switch your Bifrost mappings to use the `groups` attribute (which is already in the token) instead of `roles`. See [Attribute Mappings](`#attribute-mappings`). +If you don't want to use app roles at all, switch your Bifrost mappings to use the `groups` attribute instead of `roles` — this requires completing Step 6 (Token Claims) first. See [Attribute Mappings](`#attribute-mappings`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/enterprise/setting-up-entra.mdx` at line 421, Update the sentence mentioning the `groups` attribute to clarify that the `groups` claim is not present by default unless the user completed Step 6: Configure Token Claims; explicitly state that if they skipped Step 6 they must enable/configure the `groups` claim there before switching Bifrost mappings from `roles` to `groups`, and add a short inline link/reference to Step 6 and the Attribute Mappings section so readers know where to configure token claims.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Around line 243-247: The docs currently instruct to set
"accessTokenAcceptedVersion": 2 which uses the deprecated Azure AD Graph
manifest field; update the manifest example to the Microsoft Graph format by
replacing the root-level accessTokenAcceptedVersion with the nested
api.requestedAccessTokenVersion property (i.e., use
api.requestedAccessTokenVersion: 2 instead of accessTokenAcceptedVersion), and
adjust any surrounding text to reference api.requestedAccessTokenVersion and the
Entra admin center showing the modern manifest schema.
---
Outside diff comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Line 337: Update the sentence that incorrectly references step numbers: change
"the `groups` claim from Step 5" to "the `groups` claim from Step 6" and change
"the custom `role` claim from Step 4" to "the custom `role` claim from Step 2"
so the references to "groups claim" and "custom `role` claim" match the Token
Claims (Step 6) and Create App Roles (Step 2) sections respectively.
- Around line 362-368: Clarify the contradictory statements by specifying the
two different cases: when attribute-based role mappings are configured
(attributeRoleMappings / Role mappings list) and when no mappings exist at all;
update the "Role mappings" bullet to say that if attributeRoleMappings is
configured and no rule matches, users are not allowed to log in, and update the
Note to state that when no attributeRoleMappings are configured at all (no
mapping rules present), the first user to sign in is assigned Admin and
subsequent users receive Viewer; reference the terms "Role
mappings"/"attributeRoleMappings" and the Note about "first user Admin,
subsequent Viewer" so readers can see which behavior applies in each scenario.
---
Duplicate comments:
In `@docs/enterprise/setting-up-entra.mdx`:
- Line 421: Update the sentence mentioning the `groups` attribute to clarify
that the `groups` claim is not present by default unless the user completed Step
6: Configure Token Claims; explicitly state that if they skipped Step 6 they
must enable/configure the `groups` claim there before switching Bifrost mappings
from `roles` to `groups`, and add a short inline link/reference to Step 6 and
the Attribute Mappings section so readers know where to configure token claims.
🪄 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: 0228902b-d33e-4907-a550-7727e83924dc
📒 Files selected for processing (1)
docs/enterprise/setting-up-entra.mdx
akshaydeo
previously approved these changes
May 6, 2026
Contributor
Merge activity
|
akshaydeo
changed the base branch from
05-05-fix_mcp_tool_group_rbac_check
to
graphite-base/3237
akshaydeo
dismissed stale reviews from coderabbitai[bot] and themself
The base branch was changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improves the Microsoft Entra setup guide to fix common configuration mistakes that cause login failures and broken bulk user sync, particularly around API permission types and role assignment requirements.
Changes
openid,profile,email, andoffline_accessmust be added as Delegated permissions, whileUser.Read,User.Read.All,GroupMember.Read.All,Group.Read.All,Application.Read.All, andAppRoleAssignment.ReadWrite.Allmust be added as Application permissions. Added a warning that adding the wrong permission type causes failures even when the permission appears granted.Application.Read.AllandAppRoleAssignment.ReadWrite.Allas required Application permissions for app-role-based bulk user sync, with an explanation of why each is needed.rolesclaim, resulting in login rejection.requestedAccessTokenVersiontoaccessTokenAcceptedVersion.groupMembershipClaims: "ApplicationGroup"as a recommended manifest setting to scope thegroupsclaim to only groups assigned to the Bifrost application.Claim "roles" is not present in the tokenlogin error and one for bulk user sync assigning Viewer instead of the mapped role.Application.Read.AllandAppRoleAssignment.ReadWrite.Allare only required for app-role-based bulk sync, not for login-time role mapping.Type of change
Affected areas
How to test
Review the rendered documentation and validate against a live Microsoft Entra app registration:
rolesclaim appears in the issued ID token.Application.Read.AllandAppRoleAssignment.ReadWrite.Allare granted with admin consent.accessTokenAcceptedVersion: 2is accepted by Entra without error.Breaking changes
Related issues
Security considerations
The guide now explicitly calls out that Application permissions require admin consent to be effective, and recommends scoping the
groupsclaim to"ApplicationGroup"to avoid leaking unrelated tenant-wide group memberships into tokens.Checklist
docs/contributing/README.mdand followed the guidelines