Loosen restriction on acceptable transports?

[Stormheg]

This package validates and ignores / errors out on transports values that are not members of the AuthenticatorTransport struct. For example, during registration in the parse_registration_credential_json function, unknown transport values are ignored.

In other functions there are explicit checks that fail when confronted with unknown transport values. Like in parse_registration_options_json

The WebAuthn L2 spec (and also the L3 editors draft) mention that Relying Parties SHOULD store the transports from getTransports ^1 and warns that modifying or removing items could negatively impact the user experience. ^2

It is clear to me that py_webauthn very deliberately ignores this advice and only accepts known values, which has resulted at least two PRs to add support for new transports: https://github.com/duo-labs/py_webauthn/pull/137 and https://github.com/duo-labs/py_webauthn/pull/129

My question: what motivates this deliberate design choice? Why not follow the advice in the spec and allow unknown transports? That would seem more future proof to me.

[MasterKale]

My question: what motivates this deliberate design choice? Why not follow the advice in the spec and allow unknown transports? That would seem more future proof to me.

Hello @Stormheg, thanks for keeping an open mind about the behavior in this library. I thought it better to remove potential footguns from WebAuthn by requiring use of defined transports as defined in L2 as well as the editor's draft of L3:

https://w3c.github.io/webauthn/#enum-transport

This list has evolved over time, admittedly, and because the list changes so infrequently it hasn't felt necessary to try and make this part of the library more flexible. I usually update the library quickly enough after a new transport has been added. And recently there have been no conversations that I'm aware of to add any new transports since "hybrid" (and "smart-card" which you've reminded me I need to add.)

To your point about future-proofing...I could see the benefit in changing the type of transports here and there to List[str] instead of List[AuthenticatorTransport]. It's the browser that's tasked with providing the list after registration and parsing the list during auth, and the RP shouldn't manipulate it. I agree, then, with your assessment that my omitting unrecognized values runs afoul of that direction.

I think for now I'll leave things strongly typed. If the lists of transports starts expanding too quickly to keep up with then I could see an argument for revisiting this and making the list of transports more general. Maybe I'll make the PR to "add smart-card support" into a "generalize transports" PR instead 🤔

[Stormheg]

Thanks for getting back to me @MasterKale. I can empathize with why you made the decision to go strongly typed.

I'm still concerned that omitting unrecognized values could be an issue in the future. Yes, updating the list in py_webauthn works, but we'll be behind on the facts most of the time. It also requires all users of py_webauthn to upgrade to a new version, which is not that big of a deal but still a tad annoying.

In my opinion, treating the transports as a slightly opaque list of strings is quite acceptable given the future-proofing benefit. With this in mind, I hope you reverse your decision and make a generalized transports PR. If not that is fine with me too 🙂

For now, it is not an issue for me. Just something I noticed and wanted to know the motivation behind. Since my question is now answered, I'll go ahead and close this now 👍