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

Do not limit FIDO/U2F usage based on user agent string #6152

Closed
2 of 7 tasks
jorng opened this issue Feb 21, 2019 · 6 comments
Closed
2 of 7 tasks

Do not limit FIDO/U2F usage based on user agent string #6152

jorng opened this issue Feb 21, 2019 · 6 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality

Comments

@jorng
Copy link

jorng commented Feb 21, 2019

  • Gitea version (or commit ref): 1.7.2
  • Git version: 2.17.1
  • Operating system: Linux / Ubuntu 18.04
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant

Description

After enabling FIDO / U2F for an account on my Gitea server, I am not able to use U2F on Safari, even with the Safari FIDO U2F extension installed and enabled.

If I change my user agent in the developer settings to include Chrome, it works as expected

  • If the extension is installed/enabled, it uses U2F
  • If the extension is not installed / disabled, it asks for TOTP

Gitea should not rely on user agent string at all to determine if U2F is available. Checking that window.u2f is non-null should be enough. (for example, it works fine with Github)

@lafriks lafriks added the type/enhancement An improvement of existing functionality label Feb 21, 2019
@stale
Copy link

stale bot commented Apr 22, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Apr 22, 2019
@jorng
Copy link
Author

jorng commented Apr 22, 2019

What? Don't close.

@stale stale bot removed the issue/stale label Apr 22, 2019
@lafriks lafriks added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Apr 22, 2019
@e3b0c442
Copy link
Contributor

e3b0c442 commented Jun 4, 2019

Unfortunately the U2F support check is part of the vendored library, and looking at the latest version of that library, it still explicitly locks out Safari.

I see two ways this could be made to happen:

  1. Scrap the legacy non-standardized U2F implementations entirely and go straight to standard WebAuthn+CTAP1. The consequences of this are that we end up dropping support for Firefox 47-59, Chrome 41-66, and Opera 42-53 (per caniuse.com). I don't think this will be a huge impact since Firefox 60 ESR has this and the Chromium browsers don't have extended support releases. Chrome<67 accounts for 2.18% of total global browser usage per caniuse.com. This is the forward looking approach, and would be a first step in adopting Implement WebAuthn support #6892.

  2. Add a short-circuiting check for the window.u2f API before the call to u2fApi.ensureSupport(). This runs the risk of potentially causing issues because window.u2f is not standard. In fact, we cannot fully replace the vendored library call because Chrome itself doesn't provide window.u2f immediately. There is a U2F API library that can be retrieved from Google that implements window.u2f and could potentially replace the existing vendored library, but imho this is wasted effort given the deprecated status of unstandardized (from a web perspective) U2F implementations in favor of WebAuthn.

I'd be willing to take a stab at this if there is some agreement around one or the other option.

@DerAndereAndi
Copy link

As Safari 13.0 on macOS now has build in support for U2F and iOS 13.3 betas also including support, it would be great to make this work.

@e3b0c442
Copy link
Contributor

My work on the WebAuthn conversion PR can be tracked here: #9451

@jorng
Copy link
Author

jorng commented Dec 23, 2019

I am 💯 in favor of the migration to WebAuthn. I will close this and track that issue.

@jorng jorng closed this as completed Dec 23, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

4 participants