-
Notifications
You must be signed in to change notification settings - Fork 181
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
Require non-null userHandle when allowCredentials is empty? #1892
Comments
What might the utility be of the client-side |
@MasterKale see #558 (comment) and the answer: #558 (comment) 🙂 |
I'm going to pull those comments into here for sake of readability into conversations from 2017:
And @christiaanbrand's response:
I'm not sure I understand Christiaan's response, is the "external system with limited context" the RP's back end? I don't get how it would have "limited context" here, it's the one with the source of truth as to which credentials belong to which users. The mention of "generating unique indices" in particular doesn't make sense to me since we're talking auth, after the RP has generated the various DB indices and user ID's. |
What Christiaan means is that the credential ID is chosen by the authenticator, not the RP. The authenticator is the "external system with limited context", not the RP. So if the credential ID is the only identifier the RP can use to look up a credential, then the RP is not in control of that primary lookup index. |
Adding non-null constraints for So, I think it would be better to explicitly indicate that Also, there are some descriptions about the requirements.
|
I still don't get what exactly the problem is. Regardless of how you look up the credential record, you have to verify credentialId. I would argue that look up by credentialId, which is unique, is better from the performance perspective. Although RP doesn't generate credentialId, it inserts it in its database and checks the integrity. It has the ultimate say about credentialId and the credential record. From that perspective it is "in control of that primary lookup index". |
primary lookup index is often bound to a specific format; say a UUID. And you can't choose what the credential id format is. Especially in NoSQL databases like Cassandra or DynamoDB where the generation for the IDs need to be tightly controlled such that partitions don't create hotspots having an external system be able to generate IDs for your is bound to cause trouble. |
@arianvp also notes in #1909 (comment) :
So it looks like there's a slight mismatch between the User Handle definition (which says it's required) and the formal authenticatorGetAssertion algorithm (which doesn't explicitly say it's required). |
Proposed Change
Add language to §6.3.3. The authenticatorGetAssertion Operation to require the authenticator to return a non-null
userHandle
whenallowCredentials
is empty.Background
This was originally reported by @ionelMihai in Yubico/java-webauthn-server#94 (comment):
As far as I can tell, this authenticator implementation is against the spirit of the spec but I don't think it's against the letter of the spec. Step 15 of authenticatorGetAssertion just reads:
I didn't find any actual requirement to return a non-null
userHandle
whenallowCredentials
is empty, only that the authenticator is allowed to return null if it needs to.As noted when the user handle was introduced, the purpose of the user handle is to enable the RP to have control of how it structures its database. This purpose is undermined if some authenticators don't return a
userHandle
whenallowCredentials
is empty - the authenticator cannot know which RPs need the user handle and which don't.Therefore I believe it was intended to be a requirement that
userHandle
is non-null whenallowCredentials
is empty, in which case we should make the requirement explicit. Right now this seems to be only an implicitly understood convention.The text was updated successfully, but these errors were encountered: