Awesome work!

I have a bunch of design-related feedback:

1. The location is not correct. (You know this, and you said it.)

   • This is a system-level option, right? Yet you have it under a partition on the encryption tab. We need to promote it higher up in the hierarchy. This means it should primarily be on the top-level storage page somewhere, somehow.
   • I'm assuming that you normally would only run the check (and fix, if needed) once per system. Is there a way to save (on a system level, not in-browser) the check to know if it's been done yet or not? We could, for example, also show a message on an unchecked system but not on a passing system. (Near where it currently is, as NBDE needs support for it.)
   • Perhaps we should make something like a two-step dialog, similar to a wizard, where the first page is checking for NBDE and fixing it if we aren't sure the system supports it. The second "page" of the modal would then be using NBDE within Cockpit. If we know the system supports NBDE correctly, then we'd skip directly to the second page of the modal and only show that. (It wouldn't look like a wizard.) We do this elsewhere in Cockpit (but I don't remember where) — or at least I know I've made mockups for this concept before (perhaps this isn't implemented yet?). In this way, we wouldn't need to show any messaging about NBDE unless the user is actively going to use it.

2. Actions on modals should always be a button styled button (primary, secondary, warn, danger, etc.) and never should be a link styled button, except for "Cancel" and the cancel action must always be paired with an action button. "Close" is not "Cancel". Close always needs to be a secondary button style.

3. We probably don't need to show a passing checklist. We could show the current step and problems. And we would show a success if everything passes. We don't need to show details of things that are OK. (If we're only showing the errors and them being fixed, then it's OK to show them in an OK state after fixing them, I guess.)

4. "Fix" should probably elaborate what it is fixing, I think?

5. If it's running a medium-length process that depends on the modal being there, then you shouldn't be able to close. And you probably shouldn't be able to cancel, unless there will be no ill effects. A longer process must always allow for the modal to be closed an have status information in the page, however. (Unless everything else on the page depends on the process, such as the update process on the Software Updates page, as an example.)

I have a bunch of design-related feedback:

Thanks! This is really helpful.

This is a system-level option, right?

Mostly, yes, and we should move it up in the hierarchy. However, there are two sets of checks-and-fixes: One for the root filesystem, and another one for everything else. Currently, the buttons in the filesystem tab start one of these two flows, depending on whether the filesystem in question is the root filesystem or not.

However, the checks and fixes for a non-root filesystem are much lighter than for the root filesystem. They might not justify the full blown dialog that is implemented here. In any case, NBDE for the root filesystem is the main thing we are addressing here, so let's ignore non-root filesystems for now.

Thus, yes, this is a system-level option and should be somewhere higher up.

I'm assuming that you normally would only run the check (and fix, if needed) once per system.

Yes. However, it is always possible that the system gets broken later on. Hmm. The best would be to find a very fast reliable way to figure out if anything needs to be fixed that can be done every time the Storage page loads. I will have a try at this. (We would need to check whether the root filesystem uses NBDE, and whether the initrd includes support for this.)

However, this is starting to move us outside of the scope of Cockpit, isn't it? Isn't this something that Insights should be doing? (Checking your system in the background for things that need fixing, alerting you about them, and offering ways to actually fix them.)

Cockpit could certainly show the alerts and let people carry out the fixes, but I'd say we shouldn't go and run the actual checks in the background.

So, what if, as a first step, we only do the check-and-fix flow when adding NBDE to the root filesystem, and forget about a global button? (If I find a quick way to check whether to show this button, we can reconsider this.)

Perhaps we should make something like a two-step dialog, similar to a wizard, where the first page is checking for NBDE and fixing it if we aren't sure the system supports it.

Adding a NBDE key is already a wizard: First you enter the contact information for the server, then you need to compare fingerprints and give the final OK. In my current implementation, the check-and-fix then runs as a third step. I agree that it is much better to run it first: If the fixing fails, you should probably not add the NBDE key.

But the first step in the existing wizard includes selecting whether to add a regular passphrase, or a NBDE key. So we would have step 1: Select NBDE, enter server address; step 2: check system for NBDE support; step 3: compare and confirm fingerprints. Is that okay?

As you propose, I think we can do the check when clicking "Apply" in step 1, and immediately skip to step 3 when nothing needs fixing. Step 2 would only happen when there is something to fix.

So, what if, as a first step, we only do the check-and-fix flow when adding NBDE to the root filesystem, and forget about a global button? (If I find a quick way to check whether to show this button, we can reconsider this.)

Sure.

But the first step in the existing wizard includes selecting whether to add a regular passphrase, or a NBDE key. So we would have step 1: Select NBDE, enter server address; step 2: check system for NBDE support; step 3: compare and confirm fingerprints. Is that okay?

Wizards are for interactive input. Checking the system is not interactive; it's something that does not require input from the user. Therefore, it should not be a step in a wizard.

And this doesn't sound like it should be a wizard. A wizard is a particular UI pattern. You're talking about a process.

But as the verification step does require user input (accepting the key or not), it could be a wizard, I suppose. That's better than two sequential dialogs. But it's not as good as a dialog that changes based on context. It could be implemented as a wizard, but we don't need (nor want) a sidebar, a big header, or next/back buttons that PF wizards provide.

Here's a mockup of how it could work. It definitely needs updates and iteration.

(I made this in ExcaliDraw, so you could just open this PNG there if you want to edit it and it would show you the vector form, as it's embedded.)

Also: Since we'll check for a number of things for NBDE support and it takes a little while, the checking step could have a progress bar, as we can just divide it by # of steps and when each completes, advance the bar.

Here's a revised flow based on IRC feedback:

Minor nitpick, maybe we should have some more spacing after the SHA1 hash.

If there is no "/" this will blow up, I guess that's intendend?

It's tolerated. :-) We could produce a better error message, but this really shouldn't happen at all.