Fix NoCredentials error when checking excludeCredentials in make_credential #78
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When creating a credential (
make_credential) with anexcludeCredentialslist containing entries and using an empty credential store, the operation fails with CTAP2 error 46 (CTAP2_ERR_NO_CREDENTIALS).Common scenarios where this bug could happen:
Root cause
The
make_credentialimplementation callsfind_credentials()to check if any credentials in the exclude list exist in the store. When the store is empty,find_credentials()returnsErr(Ctap2Error::NoCredentials). The current implementation uses.await?which propagates this error, causing the credential creation to fail.However, an empty store simply means there are no credentials to exclude, which is a valid state. The credential creation should proceed normally.
The return of
Err(Ctap2Error::NoCredentials)is based on the impementation of theCredentialStoretrait, but returningErr(Ctap2Error::NoCredentials)in case there are none is a correct implementation. This could also be mitigated by handling the "no credentials" case returning an empty list, but having it return this error variant should also be compliant.Example scenario
Solution
Handle the
NoCredentialserror specifically by treating it as an empty list (no credentials to exclude), while still propagating other errors.Code changes
I think the issue comes from the changes performed in this PR: #59
The change I propose is subtle, handling the NoCredentials error specifically, as it's a valid state
To:
Tests
Added three new tests:
empty_store_with_exclude_credentials_succeeds: Verifies thatmake_credentialsucceeds when an empty store is used with a non-emptyexcludeCredentialslist. This is the primary fix test.empty_exclude_credentials_with_empty_store_succeeds: Edge case test ensuring an empty exclude list with an empty store also works correctly.store_with_credentials_not_in_exclude_list_succeeds: Verifies that when the store contains credentials but none of them are in the excludeCredentials list, credential creation should complete successfully. Added for completeness.