Skip to content

Implement basic recovery from non-terminal errors - #230

Open
msirringhaus wants to merge 2 commits into
linux-credentials:mainfrom
msirringhaus:cancellation_part3
Open

Implement basic recovery from non-terminal errors#230
msirringhaus wants to merge 2 commits into
linux-credentials:mainfrom
msirringhaus:cancellation_part3

Conversation

@msirringhaus

Copy link
Copy Markdown
Collaborator

This is a rather basic recovery strategy for the UI to restart the ceremony, if something went wrong. E.g. Hybrid aborts, then a new ceremony (with a new QR-code) is started, and the UI drops back to the start page.

Shortcomings:

  • No error message yet. This is planned for a follow-up PR because this one got quite large. So, the user currently doesn't know why something failed. If one tries to log in via USB but the device has no credential registered, for example, one gets dropped back into the start page.
  • No distinction between errors, where some error codes should remove the device from the list (like the example above. The device would still be offered on restart, even though we "know" it has no fitting credentials).
  • No "back" or "cancel"-buttons yet. Will also be part of a follow-up.
  • No UI-update while you are in "Enter PIN"-page, because the UI waits for the entry before processing other signals

Still, I think this is quite usable, if one aborts the hybrid ceremony, or unplugs the USB device mid-ceremony.

Notes:
I used 3 distinct restart-events for each transport for consistency, because we do the same for other signals.
Test are also rudimentary, to keep this PR from getting very large. I'll try to add more tests with the next (hopefully smaller) PR.

@msirringhaus
msirringhaus requested a review from iinuwa September 8, 2026 13:14
@iinuwa

iinuwa commented Sep 9, 2026

Copy link
Copy Markdown
Member

This is great! I haven't looked at the code yet, but a few notes:

Instead of adding a separate Restarted event, can we use the existing *Idle events?

If one tries to log in via USB but the device has no credential registered, for example, one gets dropped back into the start page.

If we send the device error before the transport restart event, I think the UI can reset the state in the background but leave the error page up with a back button. Or they could get fancy and transition to the start page and show a toast or something.

No distinction between errors, where some error codes should remove the device from the list (like the example above. The device would still be offered on restart, even though we "know" it has no fitting credentials).

I wonder if handling this edge case (where we know a particular authenticator cannot satisfy the request) would introduce more complexity than it value. If this part of the protocol is too tricky to implement, then no one will, but we still have to support it for backward compatibility. I think it might be fine if the user is shown an error and kicked back to the start page, and shown the same authenticators again. But I'll take a look at whatever you come up with!

@msirringhaus

msirringhaus commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Instead of adding a separate Restarted event, can we use the existing *Idle events?

From what I can tell, the Idle events are currently not sent via dbus, so we would need to add all the handlers and everything anyways (i.e. doesn't save much lines of code).
The plan was to add a payload to the restarting-signal in a follow-up PR, which gives the reason for why we had to restart. I think this is less fragile then "sending an error, followed by a restart, remember the error and act accordingly, once the restart event arrives" (see also below).
It would also be harder to distinguish between restarts that happened before the transport was actively used and after.

If we send the device error before the transport restart event, I think the UI can reset the state in the background but leave the error page up with a back button. Or they could get fancy and transition to the start page and show a toast or something.

As said above, my plan was to add a playload to the restart signal that would be displayed on the start page (probably not a toast, but a permanent text field that is invisible initially).
I would reserve the error page for terminal errors. Having an error page with a back-button would require us to stop the restarting-process until the user presses "back", because otherwise a USB-device would start blinking again, while you are still on the error page and you could interact with it, which would be very confusing. Having start page + error message, allows us to display both: "We have restarted already and here is the reason why". That's also why I'm in favor of a distinct restarting-signal which carries the restart reason as a payload.

I wonder if handling this edge case (where we know a particular authenticator cannot satisfy the request) would introduce more complexity than it value. If this part of the protocol is too tricky to implement, then no one will, but we still have to support it for backward compatibility. I think it might be fine if the user is shown an error and kicked back to the start page, and shown the same authenticators again. But I'll take a look at whatever you come up with!

Yeah, I agree. Just wanted to mention what this PR does not implement.

wdyt?

@iinuwa

iinuwa commented Sep 9, 2026

Copy link
Copy Markdown
Member

I think that's all reasonable! The more stateful we make credentialsd means less state required on those implementing the UI.

I'm just thinking about how much work this is going to be reimplementing the UI side in C for xdg-desktop-portal-gnome to get this into production, do wanted to be careful. Carry on!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants