Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove authenticatorDisplayName from L3 #2187

Closed
timcappalli opened this issue Oct 23, 2024 · 4 comments · Fixed by #2194
Closed

Remove authenticatorDisplayName from L3 #2187

timcappalli opened this issue Oct 23, 2024 · 4 comments · Fixed by #2194

Comments

@timcappalli
Copy link
Member

Discussed at TPAC as well as the 2024-10-23 call.

Relevant Issues and PRs:

@timcappalli timcappalli changed the title Remove authenticatorDisplayName from L3 in favor of adding AAGUID to credProps in L4 Remove authenticatorDisplayName from L3 Oct 23, 2024
@nadalin nadalin added this to the L3-WD-02 milestone Oct 30, 2024
@emlun
Copy link
Member

emlun commented Oct 30, 2024

2024-10-30 WG call: We've been talking about keeping credential record/authenticatorDisplayName even after deleting credProps.authenticatorDisplayName, with the motivation that it's good practice to have some kind of "nickname" for credentials. On the call it was pointed out that this is a bit odd, as this is not necessary for the protocol to work, so it seems strange to specify it so explicitly - if anything, an informative note should suffice. @emlun to open a PR to drop credential record/authenticatorDisplayName to see what the WG thinks (the 2024-10-30 call had low attendance).

@emlun emlun self-assigned this Oct 30, 2024
@zacknewman
Copy link
Contributor

zacknewman commented Oct 30, 2024

On the call it was pointed out that this is a bit odd, as this is not necessary for the protocol to work, so it seems strange to specify it so explicitly - if anything, an informative note should suffice.

To be fair, it's listed in the OPTIONAL items section; so I don't think it's "odd" at all. That same argument would suggest that the other OPTIONAL items should be removed too since they are also "not necessary for the protocol to work" in terms of storing the data. One would have to add another condition like "the data must already exist in some way" (e.g., attestationObject) or is necessary for the protocol to work.

If it's retained, then a decision would have to be made on whether it's dynamic or not. Currently in the authentication section it's allowed to change but only via the credProps extension which will no longer have it. Either that will have to be changed to reference some unnamed way for an RP to change it or it's a static value only set during registration.

@emlun
Copy link
Member

emlun commented Nov 13, 2024

Currently in the authentication section it's allowed to change but only via the credProps extension which will no longer have it.

Hm. That step was never intended to imply that it is the only allowed way to change a credential nickname. This suggests even more strongly to me that we should just delete credential record/authenticatorDisplayName altogether, and if anything just hint vaguely at the idea of allowing users to set a credential nickname.

@zacknewman
Copy link
Contributor

I think that was more of an oversight as the registration ceremony only mentions credProps as a possible additional mechanism to set authenticatorDisplayName. The authentication section should have been written similarly. Regardless, I don't care enough about this; so if people want it removed, then so be it.

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

Successfully merging a pull request may close this issue.

4 participants