Skip to content

Conversation

@msirringhaus
Copy link
Collaborator

WIP as of now.

Looking how many devices are there on the start of an operation.
If one is there, just continue with that. If none are there, got to UsbState::Waiting as before.

If more than one are there, let each of them blink in it's own tokio-task and wait for user interaction. Once the user selects one, that operation returns with true, and we send that over a dedicated channel.
If the user does not select any device, we run into timeouts, count out how many timeouts we collected and if all starting devices are accounted for, stop the operation.

I'm not yet able to stop the blinking of all not-chosen devices, once the user has selected one, because I can't clone the hid-channel, and I can't have them in the UsbState-enum, because they are not clone-able.

@msirringhaus msirringhaus changed the title WIP: Implement "Select devices" flow Implement "Select devices" flow May 19, 2025
@msirringhaus msirringhaus marked this pull request as ready for review May 19, 2025 11:03
@iinuwa
Copy link
Member

iinuwa commented May 19, 2025

I'm not yet able to stop the blinking of all not-chosen devices, once the user has selected one, because I can't clone the hid-channel, and I can't have them in the UsbState-enum, because they are not clone-able.

Have you tried wrapping the device/channel in Arc<Mutex<Option<HidDevice>>? That should allow access from multiple threads. You may need to use the Tokio version of Arc and Mutex if the lock is held across an await point to prevent deadlocks.

@iinuwa
Copy link
Member

iinuwa commented May 21, 2025

The happy path works well here: I'm able to select a specific device! If I unplug a device while in SelectingDevice state, the service crashes on https://github.com/msirringhaus/xdg-credentials-portal/blob/2350536d2e5433916c9d23efd82891dbc9366cea/libwebauthn/src/transport/hid/channel.rs#L267.

I'll create an issue over there for that.

I'm not yet able to stop the blinking of all not-chosen devices, once the user has selected one, because I can't clone the hid-channel, and I can't have them in the UsbState-enum, because they are not clone-able.

Is the above still true? For me, after selecting a device, the other one keeps blinking.

If that's expected, let's file this as a separate issue and call this one done.

@iinuwa
Copy link
Member

iinuwa commented May 21, 2025

Created linux-credentials/libwebauthn#103 for the panic on unplug issue

@iinuwa
Copy link
Member

iinuwa commented May 21, 2025

(Finally applied rustfmt to all the files; merged the conflicts back into your branch.)

@msirringhaus
Copy link
Collaborator Author

I'm not yet able to stop the blinking of all not-chosen devices, once the user has selected one, because I can't clone the hid-channel, and I can't have them in the UsbState-enum, because they are not clone-able.

Is the above still true? For me, after selecting a device, the other one keeps blinking.

If that's expected, let's file this as a separate issue and call this one done.

Yes, this is still true. The HidDevices are now cloneable, but not the channels. Additionally, libwebauthn doesn't yet have a cancel-function. This was going to be my next work item.
I hope that I can create a second channel from the cloned device and abort any ongoing operation on the hardware. But first, libwebauthn needs to implement CTAPHID_CANCEL

@iinuwa iinuwa force-pushed the multiple_devices branch from 5a34298 to 93f85c7 Compare May 22, 2025 16:41
Copy link
Member

@iinuwa iinuwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good; let's leave the blink cancellation to another PR. Thanks!

@iinuwa iinuwa merged commit 612be47 into linux-credentials:main May 22, 2025
1 check passed
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