Fix Okta IdP signing cert. by getvictor · Pull Request #37078 · fleetdm/fleet

getvictor · 2025-12-10T21:08:28Z

Fixing unreleased Okta IdP signing cert issue which prevented from setting up Okta conditional access.

Testing

QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

Confirmed that the fix is not expected to adversely impact load test results

Summary by CodeRabbit

New Features
- Enhanced the IdP signing certificate download feature with an interactive button that provides real-time loading indicators and improved error handling. Users now receive immediate feedback during the download process with helpful error messages if any issues occur.
Chores
- Removed unused import references.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

getvictor · 2025-12-10T21:08:41Z

@coderabbitai full review

coderabbitai · 2025-12-10T21:08:47Z

✅ Actions performed

Full review triggered.

codecov · 2025-12-10T21:11:50Z

Codecov Report

❌ Patch coverage is 12.50000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.92%. Comparing base (e9b8e47) to head (62ce767).
⚠️ Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
...ditionalAccessModal/OktaConditionalAccessModal.tsx	14.28%	12 Missing ⚠️
frontend/services/entities/conditional_access.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #37078      +/-   ##
==========================================
- Coverage   65.93%   65.92%   -0.01%     
==========================================
  Files        2298     2327      +29     
  Lines      184624   184695      +71     
  Branches     7752     7652     -100     
==========================================
+ Hits       121726   121756      +30     
- Misses      51762    51804      +42     
+ Partials    11136    11135       -1

Flag	Coverage Δ
frontend	`53.98% <12.50%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai · 2025-12-10T21:12:09Z

Walkthrough

A new client-side certificate download flow is introduced for the Okta Conditional Access Modal. A new API method fetches the IdP signing certificate as a blob, while the modal component replaces a static link with an interactive button that triggers the download with loading state and error handling via flash notifications.

Changes

Cohort / File(s)	Summary
Okta Conditional Access Modal `frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx`	Replaced static certificate download link with interactive Button component; added `isDownloadingCert` state and `onDownloadSigningCert` handler that fetches signing certificate via API, creates blob URL, triggers download as fleet-idp-signing-cert.pem, and cleans up URL; includes error handling with flash messaging and finally block to reset loading state
Conditional Access Service `frontend/services/entities/conditional_access.ts`	Added public method `getIdpSigningCert()` that performs GET request to CONDITIONAL_ACCESS_IDP_SIGNING_CERT endpoint with blob response type and returns the resulting Blob

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Verify proper blob URL cleanup to prevent memory leaks in onDownloadSigningCert handler
Confirm error handling integrates correctly with existing flash messaging pattern
Check loading state management during API call and download operation

Possibly related PRs

Updating UI for Okta config #35204: Overlaps with Okta conditional-access UI and endpoint additions

Suggested reviewers

jacobshandling
lukeheath
sharon-fdm

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix Okta IdP signing cert.' directly aligns with the main objective—fixing an unreleased Okta IdP signing certificate issue that prevented Okta conditional access setup.
Description check	✅ Passed	The PR description covers the issue being fixed and includes QA testing checklist items, but is missing several template sections like changes file, database migrations, and automated tests documentation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch victor/okta-signing-cert

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mostlikelee

unauthenticated routes are always going to raise red flags for me, even if they're not exposing sensitive information.

Rate limiting would help as one could DDoS the database
leaking of config info: this endpoint tells anyone if conditional access is enabled, (probably a low risk)

getvictor · 2025-12-10T23:11:05Z

unauthenticated routes are always going to raise red flags for me, even if they're not exposing sensitive information.

Rate limiting would help as one could DDoS the database

leaking of config info: this endpoint tells anyone if conditional access is enabled, (probably a low risk)

Good point. I'll work on making it work as an authenticated endpoint.

And I filed a bug on the other SAML unauthenticated endpoints: #37092

getvictor · 2025-12-10T23:40:19Z

@coderabbitai full review

coderabbitai · 2025-12-10T23:40:24Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

frontend/services/entities/conditional_access.ts (1)
35-42: IdP signing cert service method is consistent with existing patterns

This follows the same sendRequest shape as getIdpAppleProfile and correctly requests the signing cert as a blob, which fits the new download flow. Looks good as-is.

If sendRequest supports generics, you could make the return type explicit for stronger typing (optional):
-  getIdpSigningCert: (): Promise<Blob> => {
-    return sendRequest(
+  getIdpSigningCert: (): Promise<Blob> => {
+    return sendRequest<Blob>(
       "GET",
       endpoints.CONDITIONAL_ACCESS_IDP_SIGNING_CERT,
       undefined,
       "blob"
     );
   },
frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx (1)

148-166: Download handler correctly manages async blob download and loading state

The isDownloadingCert flag and onDownloadSigningCert flow look solid:

Uses the new conditionalAccessAPI.getIdpSigningCert() and treats the response as a Blob.

Creates a blob URL, triggers a download with a stable filename, and cleans up via URL.revokeObjectURL.

Properly wraps the call in a try/catch with a user-facing flash and resets loading state in finally.

If you want to be extra defensive, you could consider revoking the object URL in a setTimeout to avoid any hypothetical timing issues in older browsers, but for modern environments this is usually fine.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9267541 and e53b5de.

📒 Files selected for processing (2)

frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx (2 hunks)
frontend/services/entities/conditional_access.ts (1 hunks)

🔇 Additional comments (1)

frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx (1)

310-316: Button wiring to download handler and loading state looks correct

Hooking the “Download certificate” Button to onDownloadSigningCert and binding isLoading={isDownloadingCert} matches the new async flow and aligns with how the Save button uses isLoading. This should prevent repeated clicks during the download and keeps the UX consistent.

jacobshandling · 2025-12-11T05:17:56Z


+  const [isDownloadingCert, setIsDownloadingCert] = useState(false);
+
+  const onDownloadSigningCert = useCallback(async () => {


This all makes sense, dicey and specific stuff. If you run into more trouble here, @gillespi314 did
some similar interesting engineering in DownloadCSR that might be helpful to reference. Looks like
she has some additional memoization layers.

…components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx

jacobshandling

Assuming Tim's concerns are going to be addressed server-side this LGTM

Fixing unreleased Okta IdP signing cert issue which prevented from setting up Okta conditional access. ## 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  ## Summary by CodeRabbit * **New Features** * Enhanced the IdP signing certificate download feature with an interactive button that provides real-time loading indicators and improved error handling. Users now receive immediate feedback during the download process with helpful error messages if any issues occur. * **Chores** * Removed unused import references. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>  --------- Co-authored-by: jacobshandling <61553566+jacobshandling@users.noreply.github.com> (cherry picked from commit 0c2d465)

Fix Okta IdP signing cert.

f078d43

getvictor temporarily deployed to Docker Hub December 10, 2025 21:08 — with GitHub Actions Inactive

getvictor marked this pull request as ready for review December 10, 2025 21:21

getvictor requested review from a team as code owners December 10, 2025 21:21

getvictor assigned mostlikelee Dec 10, 2025

mostlikelee reviewed Dec 10, 2025

View reviewed changes

getvictor marked this pull request as draft December 10, 2025 23:11

Revert back to auth endpoint.

e53b5de

getvictor temporarily deployed to Docker Hub December 10, 2025 23:16 — with GitHub Actions Inactive

getvictor marked this pull request as ready for review December 10, 2025 23:39

getvictor assigned jacobshandling Dec 10, 2025

coderabbitai Bot reviewed Dec 10, 2025

View reviewed changes

jacobshandling reviewed Dec 11, 2025

View reviewed changes

Comment thread ...cards/ConditionalAccess/components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx

jacobshandling reviewed Dec 11, 2025

View reviewed changes

Update frontend/pages/admin/IntegrationsPage/cards/ConditionalAccess/…

62ce767

…components/OktaConditionalAccessModal/OktaConditionalAccessModal.tsx

jacobshandling temporarily deployed to Docker Hub December 11, 2025 05:20 — with GitHub Actions Inactive

jacobshandling self-requested a review December 11, 2025 05:21

jacobshandling approved these changes Dec 11, 2025

View reviewed changes

getvictor merged commit 0c2d465 into main Dec 11, 2025
17 checks passed

getvictor deleted the victor/okta-signing-cert branch December 11, 2025 15:04

getvictor added a commit that referenced this pull request Dec 11, 2025

Fix Okta IdP signing cert. (#37078) (#37123)

5c52717

Navigation Menu

Uh oh!

Fix Okta IdP signing cert. - #37078

Fix Okta IdP signing cert.#37078
getvictor merged 3 commits into
mainfrom
victor/okta-signing-cert

getvictor commented Dec 10, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

getvictor commented Dec 10, 2025

Uh oh!

coderabbitai Bot commented Dec 10, 2025

Uh oh!

codecov Bot commented Dec 10, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot commented Dec 10, 2025 •

edited

Loading

Uh oh!

mostlikelee left a comment

Uh oh!

getvictor commented Dec 10, 2025

Uh oh!

getvictor commented Dec 10, 2025

Uh oh!

coderabbitai Bot commented Dec 10, 2025

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

jacobshandling Dec 11, 2025

Uh oh!

jacobshandling left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		const [isDownloadingCert, setIsDownloadingCert] = useState(false);

		const onDownloadSigningCert = useCallback(async () => {

Uh oh!

Conversation

getvictor commented Dec 10, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Summary by CodeRabbit

Uh oh!

getvictor commented Dec 10, 2025

Uh oh!

coderabbitai Bot commented Dec 10, 2025

Uh oh!

codecov Bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

mostlikelee left a comment

Choose a reason for hiding this comment

Uh oh!

getvictor commented Dec 10, 2025

Uh oh!

getvictor commented Dec 10, 2025

Uh oh!

coderabbitai Bot commented Dec 10, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jacobshandling Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

jacobshandling left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

getvictor commented Dec 10, 2025 •

edited by coderabbitai Bot

Loading

codecov Bot commented Dec 10, 2025 •

edited

Loading

coderabbitai Bot commented Dec 10, 2025 •

edited

Loading