solokeys / solo1-cli

Solo 1 library and CLI in Python
https://pypi.org/project/solo-python
Apache License 2.0
182 stars 69 forks source link

Fix "make init" and "pip install solo1-cli" after upstream fido2 library change #152

Open kms15 opened 2 years ago

kms15 commented 2 years ago

The fido2 library has changed the names of the CTAP1 class to Ctap1 and the CTAP2 class to Ctap2. The old names were removed in the most recent version of the fido2 library in the python package index, which breaks installing solo1-cli with pip and breaks running "make init" with the existing solo1-cli repository.

This pull request updates the solo1-cli code to use the new names (which hopefully will be stable now that the fido2 library has reached version 1.0).

Fixes #151

vladak commented 2 years ago

Perhaps also consider fixing the fido2 library to 1.0.0 in https://github.com/solokeys/solo1-cli/blob/5b0fde3ee3d48e48a3acb83c6f21ae7f0abcffc6/pyproject.toml#L17

kms15 commented 2 years ago

Perhaps also consider fixing the fido2 library to 1.0.0 in

https://github.com/solokeys/solo1-cli/blob/5b0fde3ee3d48e48a3acb83c6f21ae7f0abcffc6/pyproject.toml#L17

Increasing the minimum fido2 library version to 1.0.0 sounds very reasonable to me. On the other hand, I've tried to limit this pull request to the minimal set of changes required to fix the issue in order to (hopefully) make it easy to approve and merge. The new names were made available with fido2 version 0.9.0 (see https://github.com/Yubico/python-fido2/releases/tag/0.9.0) even though the deprecated/old names were not removed until version fido2 version 1.0.0. Thus the current version requirements (fido2 >= 0.9.1) are sufficient for this patch to work.

vladak commented 2 years ago

Agree to keep the changes minimal. Hopefully someone will take a look at the PR soon as this makes the out-of-the-box experience sucky (I have just received brand new Solokey and keep hitting issues due to loose dependencies. #156 is another one). Anyhow, given the lack of automated testing for this Python code (at least I don't see any related Github action for this repo), the fixation of the fido2 package (fido2 == 1.0.0) would help to make it more stable. But again, "not-this-PR".

vladak commented 2 years ago

I think the title of the PR is too broad/optimistic - there are more issues with the transition to fido2 1.0.0, e.g. the following commands do not work:

and there might be more. Filed #157 to track the transition.

kms15 commented 2 years ago

Good catch - I've narrowed the pull request title to reflect what parts are fixed.

nickray commented 2 years ago

I agree it would be preferable (to reduce churn) to bump directly to fido2 1.0 in the hopes of some stability of this dependency which has been frequently changing. @kms15 would you be able + willing to work on this?

Re. tests, it's a bit indirect, but https://github.com/solokeys/fido2-tests likely exercises most of the functionality. Of course, tests local to this repo would also be appreciated.

kms15 commented 2 years ago

I'd be happy to work on updating the code to work with the fido2 1.0 API. I'll update this pull request once I have a more complete patch.