fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
3.16k stars 432 forks source link

SCEP profile stuck in pending if NDES times out #23525

Closed getvictor closed 1 day ago

getvictor commented 3 weeks ago

This is technically an unreleased bug, but we're treating it as a story since it mainly improves debuggability when the NDES server is having issues.

💥  Actual behavior

When a connection to NDES (admin or SCEP URL) times out, the SCEP profile still shows Pending and not failed.

While we're at it, let's return a good error for this permission issue: You do not have sufficient permission to enroll with SCEP. Please contact your system administrator.

image.png

🧑‍💻  Steps to reproduce

@PezHub saw this during QA, and restarting ngrok fixed the timeout to admin URL.

🕯️ More info (optional)

Fleet server cuts the connection with context canceled and does not save the error.

Demo

[Demo] Handling timeout and insufficient permission errors in NDES #23525 - Watch Video

getvictor commented 3 weeks ago

@noahtalerman @RachelElysia I added backend handling of timeouts and insufficient admin permission errors for NDES (see demo video). Should we surface these in the UI when Fleet admin is configuring NDES? If so, please provide copy.

noahtalerman commented 2 weeks ago

Hey @getvictor sorry for the delay.

Should we surface these in the UI when Fleet admin is configuring NDES? If so, please provide copy.

Yes! Easy to understand error messages are core to Fleet.

Victor, can you please take a stab at the updated copy and then ask @marko-lisica to review? Marko can provide a second set of eyes to see that we're using language/formatting that's consistent w/ other error messages.

marko-lisica commented 2 weeks ago

Hey @noahtalerman @getvictor, I wrote error messages. Let me know wdyt.

Insufficent permissions: Couldn't add. This account doesn't have sufficient permissions. Please use the account with enroll permission.

Wrong credentials: Couldn't add. Admin URL or credentials are invalid.

Timeout: Couldn't add. Request to NDES timed out. Please try again.

We usually start with Couldn't <verb (e.g. save, connect, add)>. We have some principles we follow when writing errors in handbook.-,Writing%20error%20messages,-When%20writing%20error).

getvictor commented 2 weeks ago

@marko-lisica

The insufficient permissions message looks good.

Wrong credentials. We already have the error message implemented from the original story. Invalid admin URL or credentials. Please correct and try again.

Timeout can either happen on the SCEP URL or the Admin URL -- should we have separate error messages?

marko-lisica commented 2 weeks ago

I would change one for wrong credentials too, to be more consistent.

Timeout can either happen on the SCEP URL or the Admin URL -- should we have separate error messages?

I guess we could have 2 error messages: Couldn't add. Request to NDES (SCEP URL) timed out. Please try again. Couldn't add. Request to NDES (admin URL) timed out. Please try again.

getvictor commented 2 weeks ago

Created #23813 for the frontend fixes.

PezHub commented 1 week ago

QA Notes:

confirm error handling has improved after fix. Additional testing notes here #23813

Screenshot 2024-11-18 at 8 41 15 PM

Screenshot 2024-11-18 at 8 32 36 PM

Screenshot 2024-11-18 at 8 37 54 PM

fleet-release commented 1 day ago

Pending profiles clear, With NDES time-outs known, Fleet's truth now appears.