webauthn-open-source / fido2-lib

A node.js library for performing FIDO 2.0 / WebAuthn server functionality
https://webauthn.io
MIT License
394 stars 118 forks source link

feat: supporting android and ios rp origins #121

Closed TomerDavid-Transmit closed 1 year ago

TomerDavid-Transmit commented 1 year ago

While using the lib, I noticed that the rpOrigin, as returned by the android native fido2 client attestation response, is considered invalid. Made this change for the lib to support android FacetIDs as origins. In depth explanation: https://fidoalliance.org/specs/uaf-v1.0-id-20141122/fido-appid-and-facets-v1.0-id-20141122.html#the-appid-and-facetid-assertions Also added support for the iOS FacetID prefix, as specified in the fido2 docs above. Empirically, our experiments with native iOS clients resulted with https rp origins - so the docs might be outdated, but I added the FacetID support just in case.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 :tada:

Comparison is base (aef3754) 92.95% compared to head (f4af93d) 92.97%.

:exclamation: Current head f4af93d differs from pull request most recent head 6497d56. Consider uploading reports for the commit 6497d56 to get more accurate results

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #121 +/- ## ========================================== + Coverage 92.95% 92.97% +0.01% ========================================== Files 16 16 Lines 5992 6007 +15 ========================================== + Hits 5570 5585 +15 Misses 422 422 ``` | [Impacted Files](https://codecov.io/gh/webauthn-open-source/fido2-lib/pull/121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webauthn-open-source) | Coverage Δ | | |---|---|---| | [lib/toolbox.js](https://codecov.io/gh/webauthn-open-source/fido2-lib/pull/121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webauthn-open-source#diff-bGliL3Rvb2xib3guanM=) | `90.46% <100.00%> (+0.40%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webauthn-open-source). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webauthn-open-source)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

JamesCullum commented 1 year ago

Hey @TomerDavid-Transmit thanks for the contribution! Are you trying to achieve the same thing as in https://github.com/webauthn-open-source/fido2-lib/pull/122 ?

TomerDavid-Transmit commented 1 year ago

Hey @TomerDavid-Transmit thanks for the contribution! Are you trying to achieve the same thing as in #122 ?

@JamesCullum yes, except for the prefix I think we should look for. #122 searches android: and ios: as prefixes, and I think we should search for android:apk-key-hash: and ios:bundle-id: for specificity. was just working on adding the iOS support + tests to this PR

Rimmassukhovsky commented 1 year ago

Very much needed addition!

HananArgov commented 1 year ago

looks like an encompassing code. good one :)

Hexagon commented 1 year ago

LGTM 👍 This one is slightly more complete than #122

@JamesCullum 3.3.6 or 3.4.0? I think i vote for 3.3.6.

amoskle commented 1 year ago

Nice change!

JamesCullum commented 1 year ago

@Hexagon I think 3.4.0 would make more sense as new feature, as previously we didn't support it.

Will merge and do it - thanks!