Automatically refresh certificates when they would expire

[kestred]

In PR #4, @da-x introduced a CachedCerts which fetches certificates and their corresponding expiry information, however using the expiry information to periodically refresh certificates was left as an exercise to the user.

With this pull request, Client::verify will now automatically refresh the certificates on a lazy as-needed basis.

This implementation has some draw-backs (occasional performance spikes; how occasional depends on google's expiry rate) but still offers an easy API and is (should be) faster than the Client::get_slow_unverified implementation of 0.3.0.

Future work would be to have a background thread that sleeps until a little bit before the cached certificates expire before waking up to refresh them.

[da-x]

Thanks for accepting my PR! Last week I've got it into a working condition, but giving it a few hours in a haste there are a few small corner cases I've left out:

• Verify that the certificate algorithm is indeed Algorithm::RS256 instead of assuming it.
• When kid = None, the second certificate could validate OK but while the first does not, causing a unwarranted overall failure. We should disregard the first certificate instead. I added a get_range to be similar to how Google 's Python backend client does this.

Anyway, I like what you did in this PR extending on my contribution. I've learned about boxed_local and shared this way.

A suggestion — perhaps add derive Clone on Client, or alternatively, export Cache and add a Client::new_with_cache()? This way several clients can share the underlying cache. When you process a login request, you can Clone from a shared Client, and allow the Cache to be a singleton behind the scenes. Using verify_with to achieve the same goal is possible but requires more user code to handle Arcing the certificates indirectly.

[halvko]

Aside from the comments on the code I have a small nitpic - having formatting of old code and addition of new code in the same commit made a review harder to conduct.

[halvko]

Because of the age of the PR, this function have since been deprecated

[halvko]

It seems to me like this could lead to multiple "threads" requesting new certificates at the same time if the user have multiple asyncronous verify calls:

"Thread" 1: locks the mutex, finds that certs.is_expired(), sets state to Expired(refresh_with(self.state, client)), unlocks the mutex and awaits refresh_with(self.state, client)). The scheduler schedules the next call to get_cached_or_refresh:
"Thread" 2: locks the mutex, finds RefreshState::Expired, drops the mutex, and calls refresh_with(self.state, client)

This can happen as many times as the verify function is called before a refresh_with is scheduled and completed, at which point the token will be refreshed as many times as there were calls to verify while it hadn't run.

I think it would make more sense to set the token to an updating state containing a "signal reciever", where the mutex should be dropped and the thread should await a signal, then check cache state (or just expect it to still be valid, seems unlikely, if not impossible, we weren't scheduled before the cache were invalidated again), and return the certs.

[halvko]

It seems to me the range part of this functionality isn't actually used, but I might be missing future proofing? Else the BTreeMap could be swapped with a HashMap.

[longfellowone]

Any updates on this?