Kami / python-yubico-client

Python library for validating Yubico Yubikey One Time Passwords (OTPs) based on the validation protocol version 2.0.

https://yubico-client.readthedocs.org/en/latest/

Other

83 stars 19 forks source link

Remove deprecated api server names #31

Closed nrw505 closed 4 years ago

nrw505 commented 4 years ago

api.yubico.com is now load-balanced and globally distributed. api2 through api5 are just aliases for api.yubico.com and it's no longer necessary to query them all in parallel.

codecov-io commented 4 years ago

Codecov Report

Merging #31 into master will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #31   +/-   ##
=======================================
  Coverage   87.30%   87.30%           
=======================================
  Files           6        6           
  Lines         512      512           
  Branches       78       78           
=======================================
  Hits          447      447           
  Misses         45       45           
  Partials       20       20

Impacted Files	Coverage Δ
yubico_client/yubico.py	`86.76% <100.00%> (ø)`

rmallensb commented 4 years ago

@Kami , @dainnilsson Can I ask you two to review this? We'd like to get these changes through well before the old hostnames go away.

dainnilsson commented 4 years ago

Looks fine to me.

Kami commented 4 years ago

Sorry for the delay.

The changes also look good to me as well, but I didn't have time to test it yet (need to find my YubiKey first :)).

In theory it should be fine, but a lot of the exiting code is there only to support querying multiple servers in parallel.

While cleaning up that code when it's not needed anymore would be nice, my primary concern at this point is security - to make sure those changes don't affect it negatively in any way.

@dainnilsson Perhaps you can double check the code when you get a chance?

Kami commented 4 years ago

Sadly I wasn't able to find my YubiKey, but I had a look at the code and tests again and I think we should be fine since we already have tests which cover that scenario.

In the future, we just need to clean up the code and remove support for multiple URLs since it doesn't appear to be needed anymore.

nrw505 commented 4 years ago

The thing is that there are still people out there with their own private yubikey-val servers which need query-in-parallel for HA, and we don't want to stop their stuff from working with a new version of the client library.

Kami commented 4 years ago

Thanks for the contribution. This has now been merged.

I will update changelog and publish a new version to PyPi either today or tomorrow.

Kami commented 4 years ago

v1.13.0 with this change has now been published to PyPi - https://pypi.org/project/yubico-client/1.13.0/