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

Add support for Web Authentication API #17989

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

MauricioFauth
Copy link
Member

@MauricioFauth MauricioFauth commented Dec 22, 2022

Adds a two factor authentication plugin that supports FIDO2/WebAuthn security keys.

This pull request implements a custom WebAuthn server.
The WebAuthn server doesn't support attestation and some checks are still incomplete.
The CBOR decoder is not specification complete.

References:

@codecov
Copy link

codecov bot commented Dec 22, 2022

Codecov Report

Base: 47.73% // Head: 46.49% // Decreases project coverage by -1.23% ⚠️

Coverage data is based on head (f143d2a) compared to base (d0fc1da).
Patch coverage: 44.44% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             QA_5_2   #17989      +/-   ##
============================================
- Coverage     47.73%   46.49%   -1.24%     
- Complexity    16825    16959     +134     
============================================
  Files           602      607       +5     
  Lines         71576    72313     +737     
============================================
- Hits          34164    33624     -540     
- Misses        37412    38689    +1277     
Flag Coverage Δ
dbase-extension 47.17% <43.96%> (-0.01%) ⬇️
recode-extension 47.15% <43.96%> (-0.04%) ⬇️
unit-7.2-ubuntu-latest 47.17% <43.96%> (-0.01%) ⬇️
unit-7.3-ubuntu-latest 47.66% <47.03%> (+0.04%) ⬆️
unit-7.4-ubuntu-latest 47.63% <47.03%> (-0.01%) ⬇️
unit-8.0-ubuntu-latest 47.71% <47.03%> (-0.01%) ⬇️
unit-8.1-ubuntu-latest 47.71% <47.03%> (-1.27%) ⬇️
unit-8.2-ubuntu-latest 47.72% <47.03%> (-1.25%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libraries/classes/Plugins/TwoFactor/Key.php 17.97% <0.00%> (ø)
libraries/classes/WebAuthn/CustomServer.php 0.00% <0.00%> (ø)
libraries/classes/WebAuthn/WebauthnLibServer.php 44.23% <44.23%> (ø)
libraries/classes/WebAuthn/DataStream.php 59.09% <59.09%> (ø)
libraries/classes/WebAuthn/CBORDecoder.php 94.30% <94.30%> (ø)
...asses/Controllers/JavaScriptMessagesController.php 99.63% <100.00%> (+<0.01%) ⬆️
libraries/classes/Plugins/TwoFactor/WebAuthn.php 100.00% <100.00%> (ø)
libraries/classes/TwoFactor.php 77.06% <100.00%> (+0.21%) ⬆️
libraries/classes/Config/Descriptions.php 4.28% <0.00%> (-95.72%) ⬇️
libraries/classes/Template.php 75.00% <0.00%> (-7.70%) ⬇️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MauricioFauth

This comment was marked as resolved.

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I test it once more ?

@MauricioFauth MauricioFauth force-pushed the webauthn-custom branch 2 times, most recently from aab51ee to b0c5b36 Compare January 9, 2023 20:35
@MauricioFauth MauricioFauth marked this pull request as ready for review January 9, 2023 20:36
@MauricioFauth
Copy link
Member Author

@williamdes Could you test it?

There are some things still missing that can be done in other pull requests:

  • The possibility to add more than one key.
  • Automatic migration of U2F keys to use the new plugin. For now, users that had register with the old API can use Firefox to login, deactivate the U2F plugin and enable the WebAuthn plugin to use with other browsers.

@williamdes
Copy link
Member

williamdes commented Jan 11, 2023

Two-factor authentication failed: Expected a different value than "".

With and without HTTPS on http://8.2.local/@phpmyadmin/phpMyAdmin-QA_5_2/index.php?route=/preferences/two-factor

On click on the button below "Please connect your WebAuthn/FIDO2 device. Then confirm registration on the device."

@williamdes
Copy link
Member

image
Same on Firefox

@MauricioFauth
Copy link
Member Author

MauricioFauth commented Jan 12, 2023

image Same on Firefox

I can't replicate this. Looks like there is something wrong with your JavaScript. Did you compile it and updated the cache?

@williamdes
Copy link
Member

image Same on Firefox

I can't replicate this. Looks like there is something wrong with your JavaScript. Did you compile it and updated the cache?

Ah, sorry
I forgot to do this
Will test it soon

@williamdes
Copy link
Member

This error still exists when you persist on clicking the enable button.
That said the feature works fine, awesome job ! 🎉

image

I think you can merge this PR after fixing this error so testing can be done on a larger scale

@MauricioFauth MauricioFauth force-pushed the webauthn-custom branch 2 times, most recently from 4a6aeee to c0b25b5 Compare January 16, 2023 20:08
@MauricioFauth
Copy link
Member Author

This error still exists when you persist on clicking the enable button. That said the feature works fine, awesome job ! tada

image

I think you can merge this PR after fixing this error so testing can be done on a larger scale

I'm not sure why this button is appearing to you, since it's getting hidden with JS, but I fixed the error message anyway.

@williamdes
Copy link
Member

I'm not sure why this button is appearing to you, since it's getting hidden with JS, but I fixed the error message anyway.

Now it is fixed (the error) but the button only appears when I am using HTTP now. So I can click it but at least it gives an consistent error each time.

One last thing is to add the optional package to PACKAGE_LIST on create-release.sh and use the check release excludes script but nothing should show up

Adds a two factor authentication plugin that supports FIDO2/WebAuthn
security keys.

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
@MauricioFauth MauricioFauth merged commit b8578c3 into phpmyadmin:QA_5_2 Jan 16, 2023
@MauricioFauth MauricioFauth deleted the webauthn-custom branch January 16, 2023 21:32
@MauricioFauth MauricioFauth self-assigned this Jan 16, 2023
@williamdes williamdes added this to the 5.2.1 milestone Jan 16, 2023
@williamdes williamdes added the enhancement A feature request for improving phpMyAdmin label Jan 16, 2023
@williamdes
Copy link
Member

The snapshot was re-built right now, thank you for your awesome work on this !

If possible an error should be show on chrome and not nothing when the user tries to login using the old U2F method ?

@MauricioFauth
Copy link
Member Author

The snapshot was re-built right now, thank you for your awesome work on this !

Thank you!

If possible an error should be show on chrome and not nothing when the user tries to login using the old U2F method ?

This will done in another PR.

williamdes added a commit that referenced this pull request Jan 17, 2023
Fixes: #17229
Pull-request: #17989

Signed-off-by: William Desportes <williamdes@wdes.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request for improving phpMyAdmin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants