Fixed authorization against Cesenta Docker registry authentication

[skoef]

I'm using Cesenta authentication for my private docker registry and could not make authorisation work from containerd. After digging through both the code of containerd and cesenta I fixed authorisation by adjusting two things:

• during parsing of the OAuth token, containerd expects {"access_token": "..."} while cesenta is sending {"token": "..."} (https://github.com/cesanta/docker_auth/blob/master/auth_server/server/server.go#L425). I added a field to postTokenResponse called LegacyToken (didn't really know what else to call it) and when that is set but AccessToken isn't, LegacyToken is used instead.
• Cesenta only uses username/password when Basic Authentication header is set (https://github.com/cesanta/docker_auth/blob/master/auth_server/server/server.go#L188), so I'm setting that as well.

With those 2 small fixes I could pull images from my registry.

[theopenlab-ci]

Build succeeded.

• containerd-build-arm64 : SUCCESS in 8m 50s (non-voting)

[fuweid]

HI @skoef , the CI is not happy if you don't sign off for your commit :)

https://github.com/containerd/project/blob/master/CONTRIBUTING.md#sign-your-work
Please git commit -s for your two commits , thanks!

[codecov-io]

Codecov Report

Merging #3795 into master will increase coverage by 0.01%.
The diff coverage is 70%.

@@            Coverage Diff             @@
##           master    #3795      +/-   ##
==========================================
+ Coverage      42%   42.01%   +0.01%     
==========================================
  Files         131      131              
  Lines       14589    14598       +9     
==========================================
+ Hits         6128     6134       +6     
- Misses       7547     7549       +2     
- Partials      914      915       +1

Flag	Coverage Δ
#linux	45.44% <71.42%> (+0.01%)	⬆️
#windows	36.99% <70%> (+0.02%)	⬆️

Impacted Files	Coverage Δ
remotes/docker/authorizer.go	70.4% <70%> (-0.12%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 257a749...28db191. Read the comment docs.

[theopenlab-ci]

Build succeeded.

• containerd-build-arm64 : RETRY_LIMIT in 20m 29s (non-voting)

[skoef]

I'm just now seeing that a similar fallback for "token" is done in fetchToken(). I'll do a little refactoring to make it more similar. This will also return the proper errNoToken when neither token was provided.

[theopenlab-ci]

Build succeeded.

• containerd-build-arm64 : RETRY_LIMIT in 19m 37s (non-voting)

[skoef]

Not sure why only the travis darwin CI didn't succeed. Also the Appveyor error doesn't seem really related to my change.

[estesp]

LGTM

[skoef]

Just in time I saw that SetBasicAuth() should be called áfter checking if NewRequest() resulted in an error

[dmcgowan]

Cesenta should be either implementing the OAuth endpoint or returning a 405 that it is not implemented. These changes represent an undocumented and unsupported API deviation. I would suggest first getting these changes into Cesenta since it is an open source project. After that is fixed, we can discuss adding these special cases. Like with all special cases, we heavily document the version, date, and those which do not implement the specified protocol.

[skoef]

@dmcgowan I certainly think that is fair, however, for the Token/AccessToken-part of the PR: this seemed already implemented for fetchToken but never for fetchTokenWithOAuth. Although they both fulfil a different role, the functions behave kind of similar. As there apparently was a reason to implement the fallback from access_token to token in fetchToken, why not add it to fetchTokenWithOAuth as well?

OTOH: I'm far from an OAuth-expert, let's say that as well.

[theopenlab-ci]

Build succeeded.

• containerd-build-arm64 : RETRY_LIMIT in 17m 45s (non-voting)

[dmcgowan]

@skoef the problem here is that is seems Cesanta is not implementing the POST endpoint at all, however they are also not blocking it and seem to just be handling it like a GET but form data instead of query params. The GET and POST endpoints implement a different protocol (GET was just a custom thing designed by Docker and handles anonymous auth while POST is trying to follow the OAuth specification). I think if there is going to be a workaround related to Cesanta, that workaround should be to fallback to the GET endpoint when the POST fails (this is ideally done through 405 but we have a few other special cases in there). It is important that upstream Cesanta can handle this correctly, that is what that repo is designed to do.

[dmcgowan]

@skoef what does the response look like today from the POST? If they are returning a 200 with a body that can't be handled properly, we could just fallback in that case, currently our workarounds are just related to status code but we can also consider the body.

[skoef]

I think if there is going to be a workaround related to Cesanta, that workaround should be to fallback to the GET endpoint when the POST fails (this is ideally done through 405 but we have a few other special cases in there). It is important that upstream Cesanta can handle this correctly, that is what that repo is designed to do.

You're right, I filed a PR with Cesenta's docker_auth (cesanta/docker_auth#265), we'll see what happens there!

[skoef]

The PR was merged by cesenta, so I'm closing this. Thanks anyway!