-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Update OAuth spec to add support for authorization code #1708
base: main
Are you sure you want to change the base?
Conversation
cbaf126
to
f3adb12
Compare
Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
f3adb12
to
2591150
Compare
Current coverage is 54.75%
@@ master #1708 diff @@
==========================================
Files 121 120 -1
Lines 10578 10493 -85
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 7149 5745 -1404
- Misses 3429 4240 +811
- Partials 0 508 +508
|
@dmcgowan do you have an ETA for when this feature will be GA? |
@marcellodesales the difficult part is going to be getting web login support within the docker client. I have everything else prototyped and working based on this specification. This specification will likely not get merged until it is supported within Docker which would be 1.13 at the soonest. |
- *auth_url* - Authorization endpoint to send client | ||
- *redirect_url* - Redirect location after login flow | ||
- *landing_url* - Location to redirect after login complete | ||
- *scopes* - Space separate list of scopes used by oauth provider, should be list of scopes needed registry and namespace identification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need a few examples of valid scopes for this use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://tools.ietf.org/html/rfc6749#section-3.3 and http://stackoverflow.com/questions/8449544/multiple-scope-values-to-oauth2, this should work:
scope=repository:org1/repo1:pull repository:org1/repo2:pull
But with current registry Docker-Distribution-Api-Version: registry/2.0
it does not 😞
{"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":[{"Type":"repository","Class":"","Name":"org1/repo1","Action":"pull"}]}]}
So, I need to call auth.docker.io for every repository 😞 Wildcard is also not supported (#1308)
P.S. org1
,repo1
, repo1
are example names for private repositories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aurelijusbanelis no auth.docker.io
does not support this today. We would push to get that supported if we accept this proposal.
So, I need to call auth.docker.io for every repository
On auth.docker.io
you can actually specify the "scope" parameter multiple times to get scopes for multiple repositories. This change is trying to update it to support allow multiple in the same scope value to align with the spec, but still possible today in a non-spec compliant way.
|Docker| Login |Docker| | ||
|Client| With |Daemon| +--------+ | ||
| | Creds | | |Registry| | ||
| +--------> | GET /v2/ | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make it clear that this should work on any endpoint and not just /v2/
.
This is not included in 1.13 (registry 2.6), let me tentatively add it to the 2.7 milestone |
@dmcgowan Is there an ETA for this one? We definitely would love to have this integration, as we use Github Enterprise for authentication. |
requesting an access token with a refresh token this should be set to | ||
"refresh_token". | ||
is used for authenticating to an authorization using a code given from | ||
directly from a resource owner and without having to send credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "given from directly from" -> "provided by"? And you also want to keep the "server" part of "authorization server".
Upon reviewing the updates, I notice that the spec refers to getting the authorization information by sending a HEAD request to /token. I don't know if this is correct. The RFC spec linked above does not mention a HEAD (nor even a GET) being used with /token. Also, with two OAuth2 servers I have set up, one being ADFS, the other being a custom built OAuth2/OpenIDConnect identity provider, the former appears to immediately close the connection when using HEAD on the token url, the latter response with a 400 error. Testing with various data while performing a POST doesn't seem to respond with a 401, nor show any information concerning how to challenge. That said, both identity providers have, in addition to a token endpoint, also an authorization endpoint. And, in my experience, when connecting them to a client, the client either needs to be supplied with both the token and authorization endpoint, or a metadata endpoint which contains this information. |
@TheJayMann the original proposal is kind of old. The part that was ended to be solved by the |
Summary
Add support for the
authorization_code
grant type as specified in the OAuth2 spec at https://tools.ietf.org/html/rfc6749#section-4.1. Additional changes involve communicating the necessary OAuth2 parameters to the client so that it can direct the user to the oauth provider's URL for authorization.Changes
authorization_code
grant typeWWW-Authenticate
challenge response to token endpointHEAD
request to token endpoint to get challengeGET
as a compatibility only endpointWork in Progress
This PR has been submitted before completion to get initial feedback on design. While this does add more support for the OAuth2 spec, additional clarification may be needed to specify what is specific to the token server and what differences exist here between more common OAuth2 implementations. Additionally this is specifying a non-standard use of the
WWW-Authenticate
header which may prove more confusing than necessary.