Conditional access dropdown and copy - #37338
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #37338 +/- ##
==========================================
+ Coverage 65.95% 65.98% +0.02%
==========================================
Files 2352 2353 +1
Lines 186473 185750 -723
Branches 7784 7770 -14
==========================================
- Hits 122995 122569 -426
+ Misses 52230 51990 -240
+ Partials 11248 11191 -57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis PR adds utility functions to check if conditional access is configured for Okta and Microsoft Entra, updates components to use these utilities, and makes the conditional access feature available on all Fleet instances regardless of managed cloud status. The provider text displayed in the modal now varies based on deployment type. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx (1)
28-28: LGTM! Good refactoring to use centralized utility.Replacing the manual Okta field checks with
isOktaConditionalAccessConfigured(config)reduces code duplication and improves maintainability.Also applies to: 250-250
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/interfaces/config.ts(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx(2 hunks)frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx(4 hunks)frontend/pages/policies/ManagePoliciesPage/components/ConditionalAccessModal/ConditionalAccessModal.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: getvictor
Repo: fleetdm/fleet PR: 34566
File: server/service/integration_core_test.go:7500-7511
Timestamp: 2025-10-21T16:04:18.069Z
Learning: Okta conditional access app config in Fleet is Premium-gated and supported both on-prem and in Fleet Cloud; the Cloud-only enforcement applies to the Microsoft compliance partner endpoints, not to the Okta settings.
📚 Learning: 2025-10-21T16:04:18.069Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 34566
File: server/service/integration_core_test.go:7500-7511
Timestamp: 2025-10-21T16:04:18.069Z
Learning: Okta conditional access app config in Fleet is Premium-gated and supported both on-prem and in Fleet Cloud; the Cloud-only enforcement applies to the Microsoft compliance partner endpoints, not to the Okta settings.
Applied to files:
frontend/interfaces/config.tsfrontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx
🧬 Code graph analysis (2)
frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx (1)
frontend/interfaces/config.ts (1)
isConditionalAccessConfigured(299-306)
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx (1)
frontend/interfaces/config.ts (1)
isOktaConditionalAccessConfigured(277-287)
🔇 Additional comments (6)
frontend/interfaces/config.ts (1)
276-306: LGTM! Well-designed utility functions.These utility functions properly centralize conditional access configuration checks with appropriate null-safety handling. The boolean coercion correctly treats empty strings as unconfigured.
frontend/pages/policies/ManagePoliciesPage/components/ConditionalAccessModal/ConditionalAccessModal.tsx (2)
33-33: LGTM! Clean interface extension.Adding the
providerTextprop enables dynamic messaging based on deployment type (self-hosted vs managed cloud).
135-135: LGTM! Dynamic provider text implementation.The interpolation correctly displays provider-specific guidance based on the deployment type passed from the parent component.
frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx (3)
128-132: LGTM! Correct provider text derivation.The provider text logic appropriately reflects available conditional access providers based on deployment type: "Okta" for self-hosted instances, "Okta or Microsoft Entra" for managed cloud instances.
Based on learnings, Okta is supported on both self-hosted and managed cloud, while Microsoft Entra requires proxy infrastructure only available in managed cloud.
1206-1212: LGTM! Achieves the PR objective.The conditional access option is now unconditionally included in the automations dropdown, making it visible on all Fleet instances (both self-hosted and managed cloud). The option is still appropriately disabled when Premium tier is not available or when "All teams" is selected.
This successfully addresses issue #37305 where conditional access was not available on self-hosted instances.
1396-1401: Implementation correctly uses dynamic provider text based on deployment type.The modal configuration correctly uses
isConditionalAccessConfigured(globalConfig)and passes the dynamicproviderTextderived from theisManagedCloudflag ("Okta or Microsoft Entra" for managed cloud, "Okta" otherwise). The ConditionalAccessModal component properly receives and displays this text.Verify that the displayed text and modal behavior match the Figma designs in the referenced issue.
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #37305 Resolves issue that conditional access modal was not visible for self-hosted and contained wrong copy. ## Testing - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * "Conditional access" automation option is now available to all users without license restrictions. * Enhanced conditional access detection for both Okta and Microsoft Entra identity providers. * Improved conditional access setup process with clearer provider information display based on your configuration. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit c78c63b)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #37305 Resolves issue that conditional access modal was not visible for self-hosted and contained wrong copy. ## Testing - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * "Conditional access" automation option is now available to all users without license restrictions. * Enhanced conditional access detection for both Okta and Microsoft Entra identity providers. * Improved conditional access setup process with clearer provider information display based on your configuration. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit c78c63b)
Related issue: Resolves #37305
Resolves issue that conditional access modal was not visible for self-hosted and contained wrong copy.
Testing
For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.