camallo / dkregistry-rs

A pure-Rust asynchronous library for Docker Registry API v2
Apache License 2.0
62 stars 39 forks source link

Support oauth2 compatible auth responses (uses `access_token` field, not `token`) #174

Open edwardgeorge opened 4 years ago

edwardgeorge commented 4 years ago

Adds support for more container registries, eg: Azure (which is what I am attempting to use, requiring this change).

According to the docker specs:

For compatibility with OAuth 2.0, we will also accept token under the name access_token. At least one of these fields must be specified, but both may also appear (for compatibility with older clients). When both are specified, they should be equivalent; if they differ the client's choice is undefined.

Serde doesn't support deriving a Deserialize instance with required fields that may appear under multiple names, so rather than manually implement the complex Deserialize trait here we use an intermediate struct with a simpler TryFrom implementation. No changes to the auth handling logic itself.

edwardgeorge commented 4 years ago

@steveeJ this PR is perhaps a little more controversial with using the intermediate struct. I favoured this initially as it kept it as part of deserialisation and left the auth logic untouched and ensured that the BearerAuth datatype has a non-optional token field.

An alternative approach is implemented in this branch here: https://github.com/camallo/dkregistry-rs/compare/master...edwardgeorge:access_token_getter

If you prefer that method I can overwrite the commit in this PR with the commit from this other branch? It would be good to get this support in to allow use of the library with, eg, Azure CR which i am presently using this lib against.