Skip to content

Conversation

@nsatragno
Copy link
Contributor

This patch attempts to formalize the storage interface to make it easier
to integrate with existing databases.

See issue #27

Depends on #29

This change allows users to associate multiple credentials to their
account. When performing assertions, all the credentials are sent on the
`allowList`. When registering a new credential, existing credentials are
sent on the `excludeList`.

Updated the example to show all the credentials associated to a user.

This change is not backwards compatible.
This patch attempts to formalize the storage interface to make it easier
to integrate with existing databases.

See issue strangerlabs#27
@nsatragno
Copy link
Contributor Author

Some questions that remain:

  • Should we state the size of the public key? That will depend on the algorithm.
  • Is storing it as base64url the best idea? It makes it easier to use this library without worrying about what algorithm is being used but may not be ideal.

@codecov
Copy link

codecov bot commented Jan 30, 2020

Codecov Report

Merging #31 into master will increase coverage by 12.46%.
The diff coverage is 44.82%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #31       +/-   ##
===========================================
+ Coverage   12.66%   25.13%   +12.46%     
===========================================
  Files           5        5               
  Lines         387      382        -5     
===========================================
+ Hits           49       96       +47     
+ Misses        338      286       -52
Impacted Files Coverage Δ
src/MemoryAdapter.js 18.18% <ø> (+4.84%) ⬆️
src/Webauthn.js 13.12% <0%> (-0.11%) ⬇️
src/AttestationChallengeBuilder.js 67.53% <100%> (+61.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8d63d2...0f3077c. Read the comment docs.

@Creekie1337
Copy link

Is this ever going to get merged?

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