Fix U2F registration and signing by replacing jQuery requests by fetch

[jonasfranz]

Fix #11228

[6543]

@jonasfranz can we refactor this by moving the U2F related code into web_src/js/features/webauth.js

or is ths to mouch for this bugfix?

[silverwind]

👍 to moving it to a separate file if possible.

If we expect this to work in IE11 we should include a fetch polyfill. Just install via npm and import it on top of web_src/js/polyfills.js.

[jonasfranz]

@6543 I'm currently working on a rewrite to support WebAuthn. Do you think it's worth it to rewrite this now? You can have a look on the new implementation here: https://github.com/go-gitea/gitea/compare/master...jonasfranz:feature/webauthn?expand=1

I saw that @e3b0c442 already started working on webauthn while I'm writing this comment 😂 .

[silverwind]

JS looking good.

I wonder if we can switch the vendored u2f-api to the npm version? Documentation points to it being from this fork, is the fork still necessary?

[e3b0c442]

@6543 I'm currently working on a rewrite to support WebAuthn. Do you think it's worth it to rewrite this now? You can have a look on the new implementation here: https://github.com/go-gitea/gitea/compare/master...jonasfranz:feature/webauthn?expand=1

I saw that @e3b0c442 already started working on webauthn while I'm writing this comment .

I have, and I'm hoping to get back to that in about 6 weeks when my classes are over for the summer, that said, if you're in a position to run with it go right ahead, won't hurt my feelings.

[jonasfranz]

@silverwind the npm version is not compatible with the current implementation. I would also love to use the npm version instead. But that might not be worth the effort since it will be replaced by webauthn in some point in time anyway.

[silverwind]

I wonder why package-lock.json hasn't changed. Can you run a npm install --package-lock --package-lock-only and commit the file if it changed?

[jonasfranz]

@silverwind package-lock.json didn't change since swagger-client has cross-fetch as dependency which depends on the added package. The proposed commands also didn't change the lock file.

[silverwind]

Right, I see whatwg-fetch in there so everything's fine.

[6543]

fix do not work for me :(

[jonasfranz]

@6543 can you give me more details? Are you getting an error message for example in the browser console?

[6543]

@6543 can you give me more details? Are you getting an error message for example in the browser console?

Could not read your security key. Your browser does not support U2F security keys.

System: linux 5.6.8; 76.0 (64-bit)

[6543]

same on google chrome (Version 81.0.4044.138 (Official Build) (64-bit))

[zeripath]

@6543 are you running on a localhost? Is it possible that you have to have ssl set-up?

[codecov-io]

Codecov Report

Merging #11311 into master will increase coverage by 0.00%.
The diff coverage is 33.33%.

@@           Coverage Diff            @@
##           master   #11311    +/-   ##
========================================
  Coverage   43.82%   43.82%            
========================================
  Files         607      613     +6     
  Lines       86919    87229   +310     
========================================
+ Hits        38094    38231   +137     
- Misses      44117    44276   +159     
- Partials     4708     4722    +14     

Impacted Files	Coverage Δ
models/u2f.go	63.15% <0.00%> (ø)
modules/markup/common/footnote.go	21.22% <0.00%> (ø)
modules/markup/common/linkify.go	34.69% <100.00%> (ø)
modules/notification/indexer/indexer.go	50.00% <0.00%> (-4.67%)	⬇️
services/pull/patch.go	62.93% <0.00%> (-2.80%)	⬇️
services/pull/temp_repo.go	31.62% <0.00%> (-2.57%)	⬇️
models/unit.go	41.97% <0.00%> (-2.47%)	⬇️
modules/queue/unique_queue_disk_channel.go	53.84% <0.00%> (-1.93%)	⬇️
modules/git/repo.go	50.62% <0.00%> (-1.26%)	⬇️
routers/api/v1/repo/pull_review.go	64.55% <0.00%> (-0.71%)	⬇️
... and 25 more

---

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 505e456...2ac8c73. Read the comment docs.

[6543]

@6543 are you running on a localhost? Is it possible that you have to have ssl set-up?

with self signed cert: no change :(

[CirnoT]

@6543 Make sure you type anything in nickname field, empty one is not accepted and returns 409 Conflict.

gitea/routers/user/setting/security_u2f.go

Line 21 in f9ec2f8

ctx.Error(409)

We should consider catching 409 and displaying friendly message to user, as well as making field required.

[6543]

@CirnoT if it would be that easy ... and yes it should be shown to the user that it is required

[CirnoT]

Does it still show Your browser does not support U2F security keys? For me it simply shows Could not read your security key without anything about browser, but I assume that is because I've tested it on Windows 10 1909, so that's expected.

[CirnoT]

Interestingly the code seems to be there to handle 409 -

https://github.com/go-gitea/gitea/pull/11311/files#diff-0fc7c09bb090d93aa9e9ce1e52c5a3a1R112

It doesn't seem to work however, it just shows Your browser does not support U2F security keys.

[6543]

ok message is different with ssl now Could not read your security key.

[jonasfranz]

I close this PR since @CirnoT fixed the problem with PR #11379.

[silverwind]

Should still move the u2f parts to its own file later.