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

fixing bug with New function #22

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Conversation

ryandeivert
Copy link
Contributor

@ryandeivert ryandeivert commented Jun 7, 2024

to @yawn
cc @paulbramsen

Background

The ykoath.New() function is intended to return the first yubikey, but in v1.0.5 a bug was introduced that breaks this:

Error: no suitable reader found (out of 1 readers)

Changes

The NewFromSerialList checks to see a array of any serials was provided, and is intended to return the first yubikey in the event that no serials are provided. However, that fails because the New() function always sends an empty string ("") as the serial.

This change calls the NewFromSerialList with an empty array, ensuring the first yubikey entry is returned to the caller.

This reverts the back to what existed in <v1.0.4.

@yawn
Copy link
Owner

yawn commented Jun 11, 2024

LGTM!

@ryandeivert
Copy link
Contributor Author

@yawn can you approve the workflow, merge, and release this in a 1.0.6 fix?

@yawn
Copy link
Owner

yawn commented Jun 13, 2024

Yeah, sorry for being unresponsive. @akerl took over as the primary maintainer due to my time being in short supply - I'll need to check what they did in terms of workflow and how they are releasing this nowadays. If I'm luck I can do this right now.

@yawn
Copy link
Owner

yawn commented Jun 13, 2024

Fixed CI issue in 08ac73f, merging.

@yawn yawn merged commit a3bc6a0 into yawn:master Jun 13, 2024
2 of 3 checks passed
@yawn
Copy link
Owner

yawn commented Jun 13, 2024

All done. Thanks @ryandeivert and @paulbramsen!

Ping @akerl for visibility.

@akerl
Copy link
Collaborator

akerl commented Jun 17, 2024

<3 thanks for the merge, @yawn, and sorry for introducing the regression

@ryandeivert
Copy link
Contributor Author

thanks everyone!

@ryandeivert ryandeivert deleted the ryandeivert-fix-bug branch June 19, 2024 15:04
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.

4 participants