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

resolves #702 add fido2 up/uv/pin support #705

Closed

Conversation

schaarsc
Copy link

@schaarsc schaarsc commented Jan 1, 2023

No description provided.

@rfjakob
Copy link
Owner

rfjakob commented Feb 1, 2023

Hi, a few comments:

  1. Commit message is missing (text in RFE: add fido2-with-client-pin #702 is good, put it into the commit message)
  2. You cannot change existing command-line flags as this breaks existing users
  3. The tests fail

@schaarsc
Copy link
Author

I understand the wish to avoid breaking changes. In this case however, I'd advocate for it, because a) it provides a cleaner cli and b) I assume this feature is used by tech savvy people that can handle the switch from "--fido" to "--fido-device" if this is documented in the Release Notes.
to make sure already encrypted files are not affected, I left fido2(line 50) untouched in case no parameters are provided.

As for the code comments... I'm not a go developer, consider this to be a proof of concept, in my opinion a skilled developer should take it from here and make the appropriate changes / quality checks required for this project.

@rfjakob
Copy link
Owner

rfjakob commented Mar 13, 2024

Functionality will be merged via the simpler #807

@rfjakob rfjakob closed this Mar 13, 2024
rfjakob pushed a commit that referenced this pull request Apr 19, 2024
Add an option to specify user verification options for `fido2-assert -t`

Options will be saved to config file

Provide same functionality to #705 with simpler implementation

Resolve #702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants