integreat-cms icon indicating copy to clipboard operation
integreat-cms copied to clipboard

Update webauthn to version 2.0

Open david-venhoff opened this issue 11 months ago • 2 comments

Short description

That version contains some breaking changes, and this pr changes the affected code. Since I haven't worked much with 2fa yet, it probably makes sense that someone with more experience reviews this. This pr follows the upgrade guidelines from webauthn: https://github.com/duo-labs/py_webauthn/releases/tag/v2.0.0

Proposed changes

  • Add a new bytes field webauthn_id to the user model that is automatically generated by webauthn and use it instead of the numerical user id
  • Update some renamed function calls

Side effects

/

Resolved issues

Fixes: #2614


Pull Request Review Guidelines

david-venhoff avatar Mar 28 '24 20:03 david-venhoff

Code Climate has analyzed commit 71937c1b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.2% (0.0% change).

View more on Code Climate.

codeclimate[bot] avatar Mar 28 '24 20:03 codeclimate[bot]

@deen13 I did some testing with your newly introduced test suite and it seems nice to have. However, I have no real experience with ts testing and I just wanted to ask if you think that this testing framework will work in general in case we want to extend our testing scope to TS/JS as well? :)

ulliholtgrave avatar May 16 '24 13:05 ulliholtgrave

I just wanted to ask if you think that this testing framework will work in general in case we want to extend our testing scope to TS/JS as well?

Hey @ulliholtgrave, yes vitest is a general purpose testing framework. It offers out-of-the-box support for typescript, esm and jsx which makes it the preferred testing solution for me. I hope that answers your question. 🙈

deen13 avatar May 21 '24 11:05 deen13

@deen13 In the issue grooming we talked about splitting up this pr into only upgrading webauthn and another one for the typescript tests. If you are also okay with that, could you create a new pr with only the test changes? I have already created a new branch which should have the correct changes at https://github.com/digitalfabrik/integreat-cms/compare/feature/ts_tests

david-venhoff avatar May 21 '24 14:05 david-venhoff

Perfect :) @david-venhoff Can you put your changes into one commit and this it good to go :)

ulliholtgrave avatar May 22 '24 06:05 ulliholtgrave

@david-venhoff I've created #2810 from your branch :)

deen13 avatar May 22 '24 09:05 deen13