scim flow improvements#2457
Closed
akshaydeo wants to merge 1 commit intographite-base/2457from
Closed
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds enterprise "Business Units" UI and pages, integrates an RBAC-gated sidebar entry, updates Okta setup docs for role/group mapping, introduces fallback views for Teams/Business Units, removes two test helpers, and adds minor Go doc comments and preload fix in the config store. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
|
This was referenced Apr 1, 2026
Merged
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
11 tasks
Contributor
Confidence Score: 5/5This PR is safe to merge; all previously flagged P0/P1 issues have been resolved. All blocking issues from prior review threads are addressed. The only remaining finding is a minor stale step-range reference in documentation that does not affect functionality. docs/enterprise/setting-up-okta.mdx has a minor step range inaccuracy on line 209. Important Files Changed
Reviews (7): Last reviewed commit: "scim flow improvements" | 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: 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-okta.mdx (2)
188-212:⚠️ Potential issue | 🟡 MinorRenumbering left stale step references in later instructions.
After step renumbering, references are inconsistent:
- Line 207: “skipped Steps 4-7” no longer matches the flow.
- Line 234: groups setup points to Step 6, but groups are configured in Step 5.
🧭 Suggested doc edit
-If you skipped Steps 4-7, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard. +If you skipped Steps 4-5, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard.-If you configured groups in Okta (Step 6), you can map Okta group names directly to Bifrost roles. This is an alternative to using custom role claims (Steps 4-5) and works with all Okta plans. +If you configured groups in Okta (Step 5), you can map Okta group names directly to Bifrost roles. This is an alternative to using custom role claims (Steps 3-4) and works with all Okta plans.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 188 - 212, The note under "Step 6: Assign Users to the Application" contains stale step-number references; update the phrase "skipped Steps 4-7" to the correct range (e.g., "skipped Steps 4-6") and change any instructions that say groups are configured in Step 6 to point to Step 5 instead (ensure the sentence referencing groups setup matches the actual "Step 5" where groups are configured), and scan nearby text around "Step 6: Assign Users to the Application", the Note paragraph containing "skipped Steps 4-7", and the later sentence that points groups to Step 6 to make these step-number references consistent after renumbering.
74-212:⚠️ Potential issue | 🟡 MinorFix incorrect step reference at line 234 (Group-to-Role Mappings section).
Line 234 states: "If you configured groups in Okta (Step 6)..." but groups are configured in Step 5: Configure Groups, not Step 6. Step 6 is "Assign Users to the Application" and covers user-to-role assignment, not group creation. Update the reference to Step 5.
Note: Full stack verification against PRs
#2363,#2365,#2418,#2457could not be completed in the sandbox environment (GitHub API authentication unavailable). Recommend manually verifying that step references and RBAC/group precedence behavior align with these related PRs before merge, as per the coding guidelines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 74 - 212, The step reference "If you configured groups in Okta (Step 6)..." in the Groups-to-Role Mappings section is incorrect; update that sentence to reference Step 5 ("Configure Groups") instead of Step 6, i.e., replace "Step 6" with "Step 5" for the phrase starting with "If you configured groups in Okta (Step 6)...". Also scan the surrounding headings "Step 5: Configure Groups" and "Step 6: Assign Users to the Application" to ensure no other cross-references use the wrong step numbers and adjust them consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Around line 74-77: Edit the Note under "Step 3: Create Custom Role Attribute"
to remove ambiguous phrasing and fix grammar: replace "You can use both roles
and/or groups for assigning roles to users. You can learn more about
[RBAC](/enterprise/rbac) docs. Roles takes precedence over groups in role
assignment." with a concise sentence such as "You can use roles, groups, or both
to assign permissions; roles take precedence over groups." Keep the RBAC link
intact and ensure the corrected sentence appears inside the existing <Note>
block.
---
Outside diff comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Around line 188-212: The note under "Step 6: Assign Users to the Application"
contains stale step-number references; update the phrase "skipped Steps 4-7" to
the correct range (e.g., "skipped Steps 4-6") and change any instructions that
say groups are configured in Step 6 to point to Step 5 instead (ensure the
sentence referencing groups setup matches the actual "Step 5" where groups are
configured), and scan nearby text around "Step 6: Assign Users to the
Application", the Note paragraph containing "skipped Steps 4-7", and the later
sentence that points groups to Step 6 to make these step-number references
consistent after renumbering.
- Around line 74-212: The step reference "If you configured groups in Okta (Step
6)..." in the Groups-to-Role Mappings section is incorrect; update that sentence
to reference Step 5 ("Configure Groups") instead of Step 6, i.e., replace "Step
6" with "Step 5" for the phrase starting with "If you configured groups in Okta
(Step 6)...". Also scan the surrounding headings "Step 5: Configure Groups" and
"Step 6: Assign Users to the Application" to ensure no other cross-references
use the wrong step numbers and adjust them consistently.
🪄 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: 15a0213c-f81c-41c7-ac75-4c3e875b6f86
📒 Files selected for processing (1)
docs/enterprise/setting-up-okta.mdx
akshaydeo
force-pushed
the
04-01-scim_flow_improvements
branch
from
ca18045 to
d5512c9
Compare
akshaydeo
force-pushed
the
03-29-access_profiles
branch
from
de37270 to
3e8d874
Compare
akshaydeo
force-pushed
the
04-01-scim_flow_improvements
branch
from
d5512c9 to
6aeb4b1
Compare
akshaydeo
force-pushed
the
04-01-scim_flow_improvements
branch
from
6aeb4b1 to
27628bc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
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-okta.mdx (1)
208-210:⚠️ Potential issue | 🟡 MinorIncorrect step reference after renumbering.
The reference "Steps 4-7" appears incorrect. Step 6 (Assign Users) is required for the flow to work, and Step 7 (API Token) is unrelated to role claims. The note's intent seems to be about skipping the optional role/group claim configuration, which would be Steps 3-5 (Custom Attribute, Role Claim, Groups).
✏️ Suggested fix
<Note> -Role claims are available only when you configure custom claims on your authorization server. Ensure you add role claims to your chosen authorization server (for example, `/oauth2/default`) to enable RBAC. If you skipped Steps 4-7, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard. +Role claims are available only when you configure custom claims on your authorization server. Ensure you add role claims to your chosen authorization server (for example, `/oauth2/default`) to enable RBAC. If you skipped Steps 3-5, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard. </Note>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 208 - 210, Update the Note in docs/enterprise/setting-up-okta.mdx that references "Steps 4-7": the step range is incorrect after renumbering—change the reference to the optional role/group claim configuration steps (e.g., "Steps 3-5") so it correctly points to the Custom Attribute, Role Claim, and Groups steps; locate the Note block (the <Note>...</Note> section) and replace "Steps 4-7" with the correct step numbers or wording that explicitly says "Steps 3-5 (Custom Attribute, Role Claim, Groups)".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Around line 198-200: The sentence under "For assigning role" needs minor
grammar fixes: change "role level mapping" to the hyphenated compound adjective
"role-level mapping" and make the "If" after the parenthesis lowercase ("if");
update the sentence that currently reads "For each user, set their
**bifrostRole** (If you are planning to do role level mapping):" to use
"role-level mapping" and "if" to correct the grammar.
- Around line 214-228: Update the Step 7 heading copy to read "To create an API
token, navigate to Security > API > Tokens." (add the missing article "an") and
change the second image's alt attribute (the <img> under the second <Frame>
after "Click on 'Create token'") so it is specific and not a duplicate of the
first (e.g., alt="Okta create token form" or similar) to avoid repeating the
same alt text used earlier.
- Line 256: The sentence incorrectly references "Step 6" for groups
configuration; update the text to reference "Step 5" instead. Locate the
sentence "If you configured groups in Okta (Step 6), you can map Okta group
names directly to Bifrost roles." in docs/enterprise/setting-up-okta.mdx and
replace "(Step 6)" with "(Step 5)" so it correctly points to the Groups
configuration step.
- Around line 250-254: The "Role-to-Role Mappings" header is empty; either
remove that header or add documentation explaining how to configure Okta
role-to-role mappings for Bifrost (e.g., steps mirroring the existing "Steps
4-5" for custom role claims: show how to create Okta Roles, set up role mappings
in the application integration, and map Okta role names/IDs to Bifrost role
identifiers), and ensure the section includes example claim names, sample JSON
or UI steps, and any required Bifrost configuration keys so users can follow the
full mapping process.
---
Outside diff comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Around line 208-210: Update the Note in docs/enterprise/setting-up-okta.mdx
that references "Steps 4-7": the step range is incorrect after
renumbering—change the reference to the optional role/group claim configuration
steps (e.g., "Steps 3-5") so it correctly points to the Custom Attribute, Role
Claim, and Groups steps; locate the Note block (the <Note>...</Note> section)
and replace "Steps 4-7" with the correct step numbers or wording that explicitly
says "Steps 3-5 (Custom Attribute, Role Claim, Groups)".
🪄 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: ad4fb019-a972-47a3-a47a-cbaf5c0ab418
⛔ Files ignored due to path filters (3)
docs/media/user-provisioning/okta-api-token-created.pngis excluded by!**/*.pngdocs/media/user-provisioning/okta-create-token-form.pngis excluded by!**/*.pngdocs/media/user-provisioning/okta-tokens-screen.pngis excluded by!**/*.png
📒 Files selected for processing (3)
docs/enterprise/setting-up-okta.mdxframework/configstore/rdb.goplugins/governance/test_utils.go
💤 Files with no reviewable changes (1)
- plugins/governance/test_utils.go
akshaydeo
force-pushed
the
04-01-scim_flow_improvements
branch
from
27628bc to
14434d3
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 (1)
docs/enterprise/setting-up-okta.mdx (1)
209-209:⚠️ Potential issue | 🟠 MajorStep dependency note is now logically incorrect after renumbering.
Line 209 says “If you skipped Steps 4-7…”. Step 7 is API token creation and is unrelated to role-claim availability, so this instruction is misleading.
Proposed edit
-Role claims are available only when you configure custom claims on your authorization server. Ensure you add role claims to your chosen authorization server (for example, `/oauth2/default`) to enable RBAC. If you skipped Steps 4-7, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard. +Role claims are available only when you configure custom claims on your authorization server. Ensure you add role claims to your chosen authorization server (for example, `/oauth2/default`) to enable RBAC. If you skipped role-claim setup (Steps 3-4), the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` at line 209, The sentence referencing "If you skipped Steps 4-7…" is incorrect after renumbering; update the sentence in docs/enterprise/setting-up-okta.mdx so it refers only to the steps that configure custom role claims (e.g., "If you skipped Steps 4-6…" or explicitly "If you skipped Steps 4-6 (adding custom claims)…") and remove Step 7 from that range so the guidance correctly ties role-claim availability to the claim-configuration steps rather than the API token creation step.
♻️ Duplicate comments (5)
docs/enterprise/setting-up-okta.mdx (5)
250-254:⚠️ Potential issue | 🟠 Major
Role-to-Role Mappingssection is empty.Lines 250-254 introduce a header with no actionable content; this creates a dead-end in setup flow. Either add concrete instructions or remove the header for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 250 - 254, The "Role-to-Role Mappings" section currently contains only a header with no content; either populate it with concrete setup steps or remove the header to avoid a dead-end. Update the "Role-to-Role Mappings" section under the existing headings (the "Role-to-Role Mappings" header) to include a short explanation and step-by-step instructions (e.g., purpose, prerequisites, how to create/assign mappings in Okta, example mapping), or delete the header if this feature is not documented yet so only "Group-to-Role Mappings" remains.
214-226:⚠️ Potential issue | 🟡 MinorPolish API token step text and make second image alt text specific.
Line 216 is missing “an”, and Line 225 repeats the same alt text as Line 219.
Proposed edit
-To create API token, navigate to Security > API > Tokens. +To create an API token, navigate to **Security** → **API** → **Tokens**. @@ - <img src="/media/user-provisioning/okta-create-token-form.png" alt="Okta API tokens screen" /> + <img src="/media/user-provisioning/okta-create-token-form.png" alt="Create token dialog in Okta" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 214 - 226, Fix the wording and alt text in the "Step 7: Create API token for bulk user and team sync" section: add the missing article so the sentence reads "To create an API token, navigate to Security > API > Tokens." and update the second image's alt text (currently duplicated) to a specific description such as "Okta create token form" or "Okta create API token dialog" so it is distinct from the first image alt "Okta API tokens screen"; update the alt attribute on the <img> for the second Frame accordingly.
74-77:⚠️ Potential issue | 🟡 MinorTighten RBAC note grammar and precedence wording.
Line 77 is still grammatically incorrect/ambiguous (
both roles and/or,Roles takes).Proposed edit
-You can use both roles and/or groups for assigning roles to users. You can learn more about [RBAC](/enterprise/rbac) docs. Roles takes precedence over groups in role assignment. +You can use roles, groups, or both to assign roles to users. Learn more in the [RBAC](/enterprise/rbac) docs. Roles take precedence over groups.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 74 - 77, Update the Note under "Step 3: Create Custom Role Attribute" to fix grammar and clarify precedence: replace the phrase "both roles and/or groups" with "roles and/or groups" (or simply "roles or groups") and change "Roles takes precedence over groups in role assignment." to "Roles take precedence over groups in role assignment." Ensure the revised sentence clearly states that role assignments override group assignments and keep the Note block content concise and grammatical.
198-200:⚠️ Potential issue | 🟡 MinorFix heading/copy style in the role assignment subsection.
Line 198 should be title-cased/plural, and Line 200 should use
if+role-level(hyphenated).Proposed edit
-### For assigning role +### For Assigning Roles -For each user, set their **bifrostRole** (If you are planning to do role level mapping): +For each user, set their **bifrostRole** (if you are planning to do role-level mapping):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` around lines 198 - 200, Update the subsection heading and the parenthetical copy: change the heading "For assigning role" to title-cased and plural "For Assigning Roles", and update the sentence using lowercase "if" and a hyphenated adjective so the line reads like "For each user, set their **bifrostRole** (if you are planning to do role-level mapping):". Locate these strings in the docs/enterprise/setting-up-okta.mdx file and replace them accordingly.
256-256:⚠️ Potential issue | 🟡 MinorStep references are stale after renumbering.
Line 256 references groups setup as Step 6, but groups are configured in Step 5; custom role-claim setup is Steps 3-4, not 4-5.
Proposed edit
-If you configured groups in Okta (Step 6), you can map Okta group names directly to Bifrost roles. This is an alternative to using custom role claims (Steps 4-5) and works with all Okta plans. +If you configured groups in Okta (Step 5), you can map Okta group names directly to Bifrost roles. This is an alternative to using custom role claims (Steps 3-4) and works with all Okta plans.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` at line 256, Update the sentence that references setup steps so the step numbers match the current doc ordering: change the part that says "If you configured groups in Okta (Step 6)" to "Step 5" and change "custom role claims (Steps 4-5)" to "Steps 3-4"; search the same paragraph for any other stale step references and ensure they point to the updated Step 3-5 numbering to keep the Okta group and custom role-claim references consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/dashboard/components/charts/modelFilterSelect.tsx`:
- Line 25: Remove the stray "w" text node between SelectTrigger and
SelectContent in modelFilterSelect.tsx that will render and break Radix Select
wiring; open the Select component in modelFilterSelect.tsx and delete the lone
"w" between the SelectTrigger and SelectContent elements (the same pattern
appears for both model filters used by overviewTab.tsx), then run the UI to
confirm SelectTrigger/SelectContent pairing (components/ui/select.tsx) behaves
normally.
In `@ui/components/ui/asyncMultiselect.tsx`:
- Line 470: The JSX prop override for the Input component contains a malformed
identifier "CustomI nput" which breaks the build; locate the component where
props include "Input: CustomI nput" (in asyncMultiselect.tsx) and replace the
split identifier with the correctly defined function name "CustomInput" (the
function implementation exists later as CustomInput). Ensure the prop reads
exactly Input: CustomInput so imports/refs and TypeScript/JSX parsing succeed.
---
Outside diff comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Line 209: The sentence referencing "If you skipped Steps 4-7…" is incorrect
after renumbering; update the sentence in docs/enterprise/setting-up-okta.mdx so
it refers only to the steps that configure custom role claims (e.g., "If you
skipped Steps 4-6…" or explicitly "If you skipped Steps 4-6 (adding custom
claims)…") and remove Step 7 from that range so the guidance correctly ties
role-claim availability to the claim-configuration steps rather than the API
token creation step.
---
Duplicate comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Around line 250-254: The "Role-to-Role Mappings" section currently contains
only a header with no content; either populate it with concrete setup steps or
remove the header to avoid a dead-end. Update the "Role-to-Role Mappings"
section under the existing headings (the "Role-to-Role Mappings" header) to
include a short explanation and step-by-step instructions (e.g., purpose,
prerequisites, how to create/assign mappings in Okta, example mapping), or
delete the header if this feature is not documented yet so only "Group-to-Role
Mappings" remains.
- Around line 214-226: Fix the wording and alt text in the "Step 7: Create API
token for bulk user and team sync" section: add the missing article so the
sentence reads "To create an API token, navigate to Security > API > Tokens."
and update the second image's alt text (currently duplicated) to a specific
description such as "Okta create token form" or "Okta create API token dialog"
so it is distinct from the first image alt "Okta API tokens screen"; update the
alt attribute on the <img> for the second Frame accordingly.
- Around line 74-77: Update the Note under "Step 3: Create Custom Role
Attribute" to fix grammar and clarify precedence: replace the phrase "both roles
and/or groups" with "roles and/or groups" (or simply "roles or groups") and
change "Roles takes precedence over groups in role assignment." to "Roles take
precedence over groups in role assignment." Ensure the revised sentence clearly
states that role assignments override group assignments and keep the Note block
content concise and grammatical.
- Around line 198-200: Update the subsection heading and the parenthetical copy:
change the heading "For assigning role" to title-cased and plural "For Assigning
Roles", and update the sentence using lowercase "if" and a hyphenated adjective
so the line reads like "For each user, set their **bifrostRole** (if you are
planning to do role-level mapping):". Locate these strings in the
docs/enterprise/setting-up-okta.mdx file and replace them accordingly.
- Line 256: Update the sentence that references setup steps so the step numbers
match the current doc ordering: change the part that says "If you configured
groups in Okta (Step 6)" to "Step 5" and change "custom role claims (Steps 4-5)"
to "Steps 3-4"; search the same paragraph for any other stale step references
and ensure they point to the updated Step 3-5 numbering to keep the Okta group
and custom role-claim references consistent.
🪄 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: 3b224332-e48e-4fca-b5f5-86068fc4a5f3
⛔ Files ignored due to path filters (3)
docs/media/user-provisioning/okta-api-token-created.pngis excluded by!**/*.pngdocs/media/user-provisioning/okta-create-token-form.pngis excluded by!**/*.pngdocs/media/user-provisioning/okta-tokens-screen.pngis excluded by!**/*.png
📒 Files selected for processing (5)
docs/enterprise/setting-up-okta.mdxframework/configstore/rdb.goplugins/governance/test_utils.goui/app/workspace/dashboard/components/charts/modelFilterSelect.tsxui/components/ui/asyncMultiselect.tsx
💤 Files with no reviewable changes (1)
- plugins/governance/test_utils.go
🚧 Files skipped from review as they are similar to previous changes (1)
- framework/configstore/rdb.go
akshaydeo
force-pushed
the
04-01-scim_flow_improvements
branch
from
14434d3 to
c77d479
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 (1)
docs/enterprise/setting-up-okta.mdx (1)
209-209:⚠️ Potential issue | 🟡 MinorFix stale step-range reference after renumbering.
“Skipped Steps 4-7” is now inaccurate in this context and over-broad for role-claim setup. Please narrow this to the role-claim step(s) only.
Suggested edit
-Role claims are available only when you configure custom claims on your authorization server. Ensure you add role claims to your chosen authorization server (for example, `/oauth2/default`) to enable RBAC. If you skipped Steps 4-7, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard. +Role claims are available only when you configure custom claims on your authorization server. Ensure you add role claims to your chosen authorization server (for example, `/oauth2/default`) to enable RBAC. If you skipped Step 4, the first user to sign in automatically receives the **Admin** role and can manage RBAC for all subsequent users through the Bifrost dashboard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/enterprise/setting-up-okta.mdx` at line 209, The phrase "Skipped Steps 4-7" is stale and too broad; update the sentence in the paragraph that begins "Role claims are available..." to reference the specific role-claim configuration step(s) instead (replace "If you skipped Steps 4-7" with wording like "If you skipped the role-claim configuration step(s)" or "If you skipped the role-claim setup step"), so the guidance only points to the role-claim setup rather than a range of renumbered steps.
🧹 Nitpick comments (2)
ui/app/_fallbacks/enterprise/components/user-groups/businessUnitsView.tsx (1)
7-13: Add a stable test-id prefix for this new fallback view.This view introduces a new interactive CTA path through
ContactUsView; please pass atestIdPrefixso generated selectors follow the 3-part convention.Proposed change
<ContactUsView className="mx-auto min-h-[80vh]" icon={<Users className="h-[5.5rem] w-[5.5rem]" strokeWidth={1} />} title="Unlock advanced governance" description="Manage users, business units with our enterprise-grade governance. This feature is part of the Bifrost enterprise license." readmeLink="https://docs.getbifrost.ai/enterprise/advanced-governance" + testIdPrefix="business-units-contact-view" />As per coding guidelines: “
ui/**/*.{tsx,ts}: Add new interactive UI elements withdata-testidattributes following the pattern:data-testid="<entity>-<element>-<qualifier>".”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/_fallbacks/enterprise/components/user-groups/businessUnitsView.tsx` around lines 7 - 13, The ContactUsView fallback is missing a stable testIdPrefix; update the JSX for ContactUsView in businessUnitsView.tsx to pass a testIdPrefix prop (e.g., "business-units-contactus" or similar) so generated data-testid attributes follow the 3-part convention (entity-element-qualifier) for the interactive CTA path; ensure the prefix is stable, descriptive and follows the pattern used across other components.ui/app/_fallbacks/enterprise/components/user-groups/teamsView.tsx (1)
7-13: Add a test-id prefix for the teams fallback CTA path.Please provide a
testIdPrefixto keep selectors deterministic for this newly added interactive fallback flow.Proposed change
<ContactUsView className="mx-auto min-h-[80vh]" icon={<Users className="h-[5.5rem] w-[5.5rem]" strokeWidth={1} />} title="Unlock teams governance" description="Manage teams, sync from your identity provider, and control access with enterprise-grade governance. This feature is part of the Bifrost enterprise license." readmeLink="https://docs.getbifrost.ai/enterprise/advanced-governance" + testIdPrefix="teams-contact-view" />As per coding guidelines: “
ui/**/*.{tsx,ts}: Add new interactive UI elements withdata-testidattributes following the pattern:data-testid="<entity>-<element>-<qualifier>".”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/_fallbacks/enterprise/components/user-groups/teamsView.tsx` around lines 7 - 13, The ContactUsView instance needs a deterministic test id prop; add a testIdPrefix prop to the ContactUsView call (the component named ContactUsView in teamsView.tsx) with a value following the pattern entity-element-qualifier, e.g. "teams-fallback-cta" (or "teams-cta-path" if you prefer), so selectors become data-testid="{testIdPrefix}-{element}" inside the component; update the JSX invocation to include testIdPrefix="teams-fallback-cta" to satisfy the ui/**/*.{tsx,ts} guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/components/sidebar.tsx`:
- Line 407: The sidebar is checking the wrong RBAC resource: update the
hasBusinessUnitsAccess check in sidebar.tsx to call useRbac with
RbacResource.Governance (keeping RbacOperation.View) so the nav visibility
matches the route guard in governance/layout.tsx; specifically change the
hasBusinessUnitsAccess invocation to use RbacResource.Governance to ensure
consistent access control between the menu (hasBusinessUnitsAccess) and the
route guard (governance/layout.tsx).
---
Outside diff comments:
In `@docs/enterprise/setting-up-okta.mdx`:
- Line 209: The phrase "Skipped Steps 4-7" is stale and too broad; update the
sentence in the paragraph that begins "Role claims are available..." to
reference the specific role-claim configuration step(s) instead (replace "If you
skipped Steps 4-7" with wording like "If you skipped the role-claim
configuration step(s)" or "If you skipped the role-claim setup step"), so the
guidance only points to the role-claim setup rather than a range of renumbered
steps.
---
Nitpick comments:
In `@ui/app/_fallbacks/enterprise/components/user-groups/businessUnitsView.tsx`:
- Around line 7-13: The ContactUsView fallback is missing a stable testIdPrefix;
update the JSX for ContactUsView in businessUnitsView.tsx to pass a testIdPrefix
prop (e.g., "business-units-contactus" or similar) so generated data-testid
attributes follow the 3-part convention (entity-element-qualifier) for the
interactive CTA path; ensure the prefix is stable, descriptive and follows the
pattern used across other components.
In `@ui/app/_fallbacks/enterprise/components/user-groups/teamsView.tsx`:
- Around line 7-13: The ContactUsView instance needs a deterministic test id
prop; add a testIdPrefix prop to the ContactUsView call (the component named
ContactUsView in teamsView.tsx) with a value following the pattern
entity-element-qualifier, e.g. "teams-fallback-cta" (or "teams-cta-path" if you
prefer), so selectors become data-testid="{testIdPrefix}-{element}" inside the
component; update the JSX invocation to include
testIdPrefix="teams-fallback-cta" to satisfy the ui/**/*.{tsx,ts} guideline.
🪄 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: 5d8ba674-59b7-4a23-ab16-26388fd2c4b1
⛔ Files ignored due to path filters (3)
docs/media/user-provisioning/okta-api-token-created.pngis excluded by!**/*.pngdocs/media/user-provisioning/okta-create-token-form.pngis excluded by!**/*.pngdocs/media/user-provisioning/okta-tokens-screen.pngis excluded by!**/*.png
📒 Files selected for processing (9)
docs/enterprise/setting-up-okta.mdxframework/configstore/rdb.goplugins/governance/test_utils.goui/app/_fallbacks/enterprise/components/user-groups/businessUnitsView.tsxui/app/_fallbacks/enterprise/components/user-groups/teamsView.tsxui/app/workspace/dashboard/components/charts/modelFilterSelect.tsxui/app/workspace/governance/business-units/page.tsxui/app/workspace/governance/teams/page.tsxui/components/sidebar.tsx
💤 Files with no reviewable changes (1)
- plugins/governance/test_utils.go
✅ Files skipped from review due to trivial changes (2)
- ui/app/workspace/governance/business-units/page.tsx
- ui/app/workspace/dashboard/components/charts/modelFilterSelect.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- framework/configstore/rdb.go
akshaydeo
changed the base branch from
03-29-access_profiles
to
graphite-base/2457
akshaydeo
force-pushed
the
graphite-base/2457
branch
from
3e8d874 to
5f285e3
Compare
akshaydeo
force-pushed
the
04-01-scim_flow_improvements
branch
from
c77d479 to
fedd363
Compare
akshaydeo
changed the base branch from
graphite-base/2457
to
03-29-access_profiles
akshaydeo
changed the base branch from
03-29-access_profiles
to
graphite-base/2457
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
Streamlined the Okta setup documentation by removing the optional authorization server configuration step and consolidating the groups claim configuration to use only the authorization server approach.
Changes
Type of change
Affected areas
How to test
Review the updated documentation to ensure:
Screenshots/Recordings
N/A - Documentation changes only
Breaking changes
Related issues
N/A
Security considerations
No security implications - this is a documentation clarification that removes potentially confusing configuration options.
Checklist
docs/contributing/README.mdand followed the guidelines