Updating UI for Okta config - #35204
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #35204 +/- ##
==========================================
- Coverage 66.23% 66.13% -0.10%
==========================================
Files 2110 2104 -6
Lines 179360 177620 -1740
Branches 7486 7509 +23
==========================================
- Hits 118797 117470 -1327
+ Misses 49666 49406 -260
+ Partials 10897 10744 -153
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:
|
de0217c to
ddf66b4
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis PR adds Okta conditional access configuration support to the Fleet UI, complementing the existing Microsoft Entra integration. Changes include new Okta configuration fields in types and mocks, refactored ConditionalAccess component supporting dual-vendor UI, new modal components for Okta and Entra configuration flows, expanded test coverage, and a new API endpoint constant. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ConditionalAccess UI
participant EntraModal as Entra Modal
participant OktaModal as Okta Modal
participant Backend as Fleet Backend
participant OAuth as OAuth Provider
User->>UI: Select provider to configure
alt Microsoft Entra Flow
User->>EntraModal: Open Entra modal
User->>EntraModal: Enter tenant ID
User->>EntraModal: Click Save
EntraModal->>Backend: triggerMicrosoftConditionalAccess(tenantId)
Backend-->>EntraModal: Return OAuth URL
EntraModal->>OAuth: Open window to OAuth provider
OAuth-->>User: Complete OAuth flow
User->>EntraModal: Close modal
EntraModal->>UI: onSuccess callback
UI->>UI: Show "awaiting configuration" banner
else Okta Flow
User->>OktaModal: Open Okta modal
User->>OktaModal: Fill IdP ID, ACS URL, Audience URI
User->>OktaModal: Upload PEM certificate
OktaModal->>OktaModal: Validate form & file
User->>OktaModal: Click Save
OktaModal->>Backend: configAPI.update(conditional_access config)
Backend-->>OktaModal: Return updated config
OktaModal->>UI: onSuccess with updated config
UI->>UI: Show success/error banner
end
UI->>UI: Refresh provider cards display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35–45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.storybook/main.ts(1 hunks)frontend/__mocks__/configMock.ts(1 hunks)frontend/interfaces/config.ts(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.stories.tsx(2 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tests.tsx(6 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/ConditionalAccess.tsx(6 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/_styles.scss(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/EntraConditionalAccessModal/EntraConditionalAccessModal.tsx(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/EntraConditionalAccessModal/_styles.scss(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/EntraConditionalAccessModal/index.ts(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx(1 hunks)frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/index.ts(1 hunks)frontend/utilities/endpoints.ts(1 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/utilities/endpoints.tsfrontend/interfaces/config.ts
🔇 Additional comments (8)
frontend/interfaces/config.ts (1)
158-167: LGTM! Clean interface extension for Okta support.The new Okta fields are properly typed and follow the same pattern as the existing Microsoft Entra fields. The structure supports the dual-vendor conditional access UI introduced in this PR.
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/EntraConditionalAccessModal/_styles.scss (1)
1-16: LGTM! Standard modal styling.The styling uses appropriate design system variables and follows consistent SCSS patterns. The nested structure and spacing values are appropriate for the modal UI.
frontend/__mocks__/configMock.ts (1)
118-121: LGTM! Mock data aligns with interface.The new Okta fields in the mock configuration correctly match the interface updates, with appropriate empty string defaults for unconfigured state.
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/_styles.scss (1)
1-12: LGTM! Appropriate styling integration.The import statement correctly integrates the Entra modal styles, and the new flex column layout with gap provides consistent spacing for the multi-provider card UI.
.storybook/main.ts (1)
49-49: LGTM! Story coverage extension.Adding the IntegrationsPage story path appropriately extends Storybook coverage to include the new Conditional Access modals and related components.
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/EntraConditionalAccessModal/index.ts (1)
1-1: LGTM! Standard export pattern.The re-export follows conventional patterns for component index files, enabling clean imports of the EntraConditionalAccessModal component.
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/index.ts (1)
1-1: LGTM! Standard export pattern.The re-export follows the same conventional pattern as the EntraConditionalAccessModal, enabling clean imports of the OktaConditionalAccessModal component.
frontend/utilities/endpoints.ts (1)
21-21: No changes needed—endpoint path is correct.The frontend endpoint uses underscores (
conditional_access/idp/signing_cert), which matches the backend route definition atserver/service/handler.go:560and the integration test atee/server/integrationtest/condaccess/condaccess_test.go:424. The inconsistency with the Microsoft conditional access endpoint (which uses hyphens) is expected, as these are separate endpoints with different naming conventions in the backend.Likely an incorrect or invalid review comment.
GitHub Actions
Error
GitHub Actions
Inactive
| </p> | ||
| <p> | ||
| If you don't, end users will stay blocked even after deleting | ||
| Entra. |
There was a problem hiding this comment.
@noahtalerman the above copy you just added to main needs updating in the context of this new Okta conditional access option.
Here's the current Figma that Victor implemented , please update that so we can implement it accordingly here, thank you
There was a problem hiding this comment.
(test failure is related to this)
There was a problem hiding this comment.
Up to @rachaelshaw
The reason we updated the copy for Entra was because it wasn't true.
Is the current Okta copy correct? Does disconnecting Fleet from Okta unblock end users?
There was a problem hiding this comment.
I updated this to show your new copy for Entra and the current Figma for Okta
There was a problem hiding this comment.
@noahtalerman
For Okta, the connection mainly happens in Okta's settings. IT admin would need to remove Fleet as a factor to unblock users. Conversely, just because Okta appears configured in the Fleet UI does not mean Okta will actually use Fleet as a factor for conditional access. Another way to unblock users is to remove policies from conditional access.
There was a problem hiding this comment.
IT admin would need to remove Fleet as a factor to unblock users
Ok, then I think we want to update the copy. I opened a PR: #36133
Deleting Okta from Fleet won't unblock users.
Another way to unblock users is to remove policies from conditional access.
This requires a refetch before end users are unblocked right? That's why I think we want to go w/ instructions to, in Okta, remove Fleet as a factor.
FYI @rachaelshaw
GitHub Actions
Inactive
Deleting Okta from Fleet won't unblock users. New website redirect is in a separate PR: https://github.com/fleetdm/fleet/pull/36015/files#diff-0c6120927d3e65309562b5b15b261d3298d05fcb0ff18e5b6512ee683b7ad6b3 Context: #35204 (comment)
3 participants
Related issue: Resolves #34539
Figma: https://www.figma.com/design/OgQ8SyLK8Sw5thXtF1eiNP/-31909-Conditional-access-w--Okta
Requires backend PR #35526 to view Apple profile.
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)Testing
Summary by CodeRabbit