From 5a7dab46709ac51c96abaa0108afef59e01235d5 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 21 May 2015 11:14:46 -0700 Subject: [PATCH 1/4] Refactor client auth Move client auth into a separate package. Separate ping from the authorizer and export Challenges type. Signed-off-by: Derek McGowan (github: dmcgowan) --- .../{transport => auth}/authchallenge.go | 52 +++++++-- .../{transport => auth}/authchallenge_test.go | 4 +- .../client/{transport => auth}/session.go | 102 +++++------------- .../{transport => auth}/session_test.go | 55 ++++++---- 4 files changed, 109 insertions(+), 104 deletions(-) rename registry/client/{transport => auth}/authchallenge.go (67%) rename registry/client/{transport => auth}/authchallenge_test.go (95%) rename registry/client/{transport => auth}/session.go (69%) rename registry/client/{transport => auth}/session_test.go (84%) diff --git a/registry/client/transport/authchallenge.go b/registry/client/auth/authchallenge.go similarity index 67% rename from registry/client/transport/authchallenge.go rename to registry/client/auth/authchallenge.go index fffd560b..e3abfb11 100644 --- a/registry/client/transport/authchallenge.go +++ b/registry/client/auth/authchallenge.go @@ -1,6 +1,7 @@ -package transport +package auth import ( + "fmt" "net/http" "strings" ) @@ -8,10 +9,13 @@ import ( // Octet types from RFC 2616. type octetType byte -// authorizationChallenge carries information -// from a WWW-Authenticate response header. -type authorizationChallenge struct { - Scheme string +// Challenge carries information from a WWW-Authenticate response header. +// See RFC 2617. +type Challenge struct { + // Scheme is the auth-scheme according to RFC 2617 + Scheme string + + // Parameters are the auth-params according to RFC 2617 Parameters map[string]string } @@ -54,12 +58,44 @@ func init() { } } -func parseAuthHeader(header http.Header) map[string]authorizationChallenge { - challenges := map[string]authorizationChallenge{} +// Ping pings the provided endpoint to determine its required authorization challenges. +// If a version header is provided, the versions will be returned. +func Ping(client *http.Client, endpoint, versionHeader string) ([]Challenge, []string, error) { + req, err := http.NewRequest("GET", endpoint, nil) + if err != nil { + return nil, nil, err + } + + resp, err := client.Do(req) + if err != nil { + return nil, nil, err + } + defer resp.Body.Close() + + versions := []string{} + if versionHeader != "" { + for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey(versionHeader)] { + versions = append(versions, strings.Fields(supportedVersions)...) + } + } + + if resp.StatusCode == http.StatusUnauthorized { + // Parse the WWW-Authenticate Header and store the challenges + // on this endpoint object. + return parseAuthHeader(resp.Header), versions, nil + } else if resp.StatusCode != http.StatusOK { + return nil, versions, fmt.Errorf("unable to get valid ping response: %d", resp.StatusCode) + } + + return nil, versions, nil +} + +func parseAuthHeader(header http.Header) []Challenge { + challenges := []Challenge{} for _, h := range header[http.CanonicalHeaderKey("WWW-Authenticate")] { v, p := parseValueAndParams(h) if v != "" { - challenges[v] = authorizationChallenge{Scheme: v, Parameters: p} + challenges = append(challenges, Challenge{Scheme: v, Parameters: p}) } } return challenges diff --git a/registry/client/transport/authchallenge_test.go b/registry/client/auth/authchallenge_test.go similarity index 95% rename from registry/client/transport/authchallenge_test.go rename to registry/client/auth/authchallenge_test.go index 45c932b9..9b6a5adc 100644 --- a/registry/client/transport/authchallenge_test.go +++ b/registry/client/auth/authchallenge_test.go @@ -1,4 +1,4 @@ -package transport +package auth import ( "net/http" @@ -13,7 +13,7 @@ func TestAuthChallengeParse(t *testing.T) { if len(challenges) != 1 { t.Fatalf("Unexpected number of auth challenges: %d, expected 1", len(challenges)) } - challenge := challenges["bearer"] + challenge := challenges[0] if expected := "bearer"; challenge.Scheme != expected { t.Fatalf("Unexpected scheme: %s, expected: %s", challenge.Scheme, expected) diff --git a/registry/client/transport/session.go b/registry/client/auth/session.go similarity index 69% rename from registry/client/transport/session.go rename to registry/client/auth/session.go index 90c8082c..5512a9a1 100644 --- a/registry/client/transport/session.go +++ b/registry/client/auth/session.go @@ -1,4 +1,4 @@ -package transport +package auth import ( "encoding/json" @@ -9,6 +9,8 @@ import ( "strings" "sync" "time" + + "github.com/docker/distribution/registry/client/transport" ) // AuthenticationHandler is an interface for authorizing a request from @@ -32,71 +34,24 @@ type CredentialStore interface { // NewAuthorizer creates an authorizer which can handle multiple authentication // schemes. The handlers are tried in order, the higher priority authentication -// methods should be first. -func NewAuthorizer(transport http.RoundTripper, handlers ...AuthenticationHandler) RequestModifier { - return &tokenAuthorizer{ - challenges: map[string]map[string]authorizationChallenge{}, +// methods should be first. The challengeMap holds a list of challenges for +// a given root API endpoint (for example "https://registry-1.docker.io/v2/"). +func NewAuthorizer(challengeMap map[string][]Challenge, handlers ...AuthenticationHandler) transport.RequestModifier { + return &endpointAuthorizer{ + challenges: challengeMap, handlers: handlers, - transport: transport, } } -type tokenAuthorizer struct { - challenges map[string]map[string]authorizationChallenge +type endpointAuthorizer struct { + challenges map[string][]Challenge handlers []AuthenticationHandler transport http.RoundTripper } -func (ta *tokenAuthorizer) ping(endpoint string) (map[string]authorizationChallenge, error) { - req, err := http.NewRequest("GET", endpoint, nil) - if err != nil { - return nil, err - } - - client := &http.Client{ - Transport: ta.transport, - // Ping should fail fast - Timeout: 5 * time.Second, - } - - resp, err := client.Do(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - // TODO(dmcgowan): Add version string which would allow skipping this section - var supportsV2 bool -HeaderLoop: - for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey("Docker-Distribution-API-Version")] { - for _, versionName := range strings.Fields(supportedVersions) { - if versionName == "registry/2.0" { - supportsV2 = true - break HeaderLoop - } - } - } - - if !supportsV2 { - return nil, fmt.Errorf("%s does not appear to be a v2 registry endpoint", endpoint) - } - - if resp.StatusCode == http.StatusUnauthorized { - // Parse the WWW-Authenticate Header and store the challenges - // on this endpoint object. - return parseAuthHeader(resp.Header), nil - } else if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("unable to get valid ping response: %d", resp.StatusCode) - } - - return nil, nil -} - -func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error { +func (ea *endpointAuthorizer) ModifyRequest(req *http.Request) error { v2Root := strings.Index(req.URL.Path, "/v2/") - // Test if /v2/ does not exist or not at beginning - // TODO(dmcgowan) support v2 endpoints which have a prefix before /v2/ - if v2Root == -1 || v2Root > 0 { + if v2Root == -1 { return nil } @@ -108,19 +63,16 @@ func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error { pingEndpoint := ping.String() - challenges, ok := ta.challenges[pingEndpoint] + challenges, ok := ea.challenges[pingEndpoint] if !ok { - var err error - challenges, err = ta.ping(pingEndpoint) - if err != nil { - return err - } - ta.challenges[pingEndpoint] = challenges + return nil } - for _, handler := range ta.handlers { - challenge, ok := challenges[handler.Scheme()] - if ok { + for _, handler := range ea.handlers { + for _, challenge := range challenges { + if challenge.Scheme != handler.Scheme() { + continue + } if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil { return err } @@ -133,7 +85,7 @@ func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error { type tokenHandler struct { header http.Header creds CredentialStore - scope TokenScope + scope tokenScope transport http.RoundTripper tokenLock sync.Mutex @@ -141,25 +93,29 @@ type tokenHandler struct { tokenExpiration time.Time } -// TokenScope represents the scope at which a token will be requested. +// tokenScope represents the scope at which a token will be requested. // This represents a specific action on a registry resource. -type TokenScope struct { +type tokenScope struct { Resource string Scope string Actions []string } -func (ts TokenScope) String() string { +func (ts tokenScope) String() string { return fmt.Sprintf("%s:%s:%s", ts.Resource, ts.Scope, strings.Join(ts.Actions, ",")) } // NewTokenHandler creates a new AuthenicationHandler which supports // fetching tokens from a remote token server. -func NewTokenHandler(transport http.RoundTripper, creds CredentialStore, scope TokenScope) AuthenticationHandler { +func NewTokenHandler(transport http.RoundTripper, creds CredentialStore, scope string, actions ...string) AuthenticationHandler { return &tokenHandler{ transport: transport, creds: creds, - scope: scope, + scope: tokenScope{ + Resource: "repository", + Scope: scope, + Actions: actions, + }, } } diff --git a/registry/client/transport/session_test.go b/registry/client/auth/session_test.go similarity index 84% rename from registry/client/transport/session_test.go rename to registry/client/auth/session_test.go index 374d6e79..f16836da 100644 --- a/registry/client/transport/session_test.go +++ b/registry/client/auth/session_test.go @@ -1,4 +1,4 @@ -package transport +package auth import ( "encoding/base64" @@ -8,6 +8,7 @@ import ( "net/url" "testing" + "github.com/docker/distribution/registry/client/transport" "github.com/docker/distribution/testutil" ) @@ -67,17 +68,6 @@ func TestEndpointAuthorizeToken(t *testing.T) { repo2 := "other/registry" scope1 := fmt.Sprintf("repository:%s:pull,push", repo1) scope2 := fmt.Sprintf("repository:%s:pull,push", repo2) - tokenScope1 := TokenScope{ - Resource: "repository", - Scope: repo1, - Actions: []string{"pull", "push"}, - } - tokenScope2 := TokenScope{ - Resource: "repository", - Scope: repo2, - Actions: []string{"pull", "push"}, - } - tokenMap := testutil.RequestResponseMap([]testutil.RequestResponseMapping{ { Request: testutil.Request{ @@ -122,7 +112,14 @@ func TestEndpointAuthorizeToken(t *testing.T) { e, c := testServerWithAuth(m, authenicate, validCheck) defer c() - transport1 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, nil, tokenScope1))) + challenges1, _, err := Ping(&http.Client{}, e+"/v2/", "") + if err != nil { + t.Fatal(err) + } + challengeMap1 := map[string][]Challenge{ + e + "/v2/": challenges1, + } + transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap1, NewTokenHandler(nil, nil, repo1, "pull", "push"))) client := &http.Client{Transport: transport1} req, _ := http.NewRequest("GET", e+"/v2/hello", nil) @@ -141,7 +138,14 @@ func TestEndpointAuthorizeToken(t *testing.T) { e2, c2 := testServerWithAuth(m, authenicate, badCheck) defer c2() - transport2 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, nil, tokenScope2))) + challenges2, _, err := Ping(&http.Client{}, e+"/v2/", "") + if err != nil { + t.Fatal(err) + } + challengeMap2 := map[string][]Challenge{ + e + "/v2/": challenges2, + } + transport2 := transport.NewTransport(nil, NewAuthorizer(challengeMap2, NewTokenHandler(nil, nil, repo2, "pull", "push"))) client2 := &http.Client{Transport: transport2} req, _ = http.NewRequest("GET", e2+"/v2/hello", nil) @@ -166,11 +170,6 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) { scope := fmt.Sprintf("repository:%s:pull,push", repo) username := "tokenuser" password := "superSecretPa$$word" - tokenScope := TokenScope{ - Resource: "repository", - Scope: repo, - Actions: []string{"pull", "push"}, - } tokenMap := testutil.RequestResponseMap([]testutil.RequestResponseMapping{ { @@ -216,7 +215,14 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) { password: password, } - transport1 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, creds, tokenScope), NewBasicHandler(creds))) + challenges, _, err := Ping(&http.Client{}, e+"/v2/", "") + if err != nil { + t.Fatal(err) + } + challengeMap := map[string][]Challenge{ + e + "/v2/": challenges, + } + transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap, NewTokenHandler(nil, creds, repo, "pull", "push"), NewBasicHandler(creds))) client := &http.Client{Transport: transport1} req, _ := http.NewRequest("GET", e+"/v2/hello", nil) @@ -256,7 +262,14 @@ func TestEndpointAuthorizeBasic(t *testing.T) { password: password, } - transport1 := NewTransport(nil, NewAuthorizer(nil, NewBasicHandler(creds))) + challenges, _, err := Ping(&http.Client{}, e+"/v2/", "") + if err != nil { + t.Fatal(err) + } + challengeMap := map[string][]Challenge{ + e + "/v2/": challenges, + } + transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap, NewBasicHandler(creds))) client := &http.Client{Transport: transport1} req, _ := http.NewRequest("GET", e+"/v2/hello", nil) From c8fac94617d72197223490e0d57662587699fc58 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 15 Jun 2015 16:10:48 -0700 Subject: [PATCH 2/4] Separate version and challenge parsing from ping Replace ping logic with individual functions to extract API version and authorization challenges. The response from a ping operation can be passed into these function. If an error occurs in parsing, the version or challenge will not be used. Sending the ping request is the responsibility of the caller. APIVersion has been converted from a string to a structure type. A parse function was added to convert from string to the structure type. Signed-off-by: Derek McGowan (github: dmcgowan) --- registry/client/auth/api_version.go | 58 +++++++++++++++++++++++++++ registry/client/auth/authchallenge.go | 38 +++++------------- registry/client/auth/session_test.go | 43 +++++++++++++++++--- 3 files changed, 104 insertions(+), 35 deletions(-) create mode 100644 registry/client/auth/api_version.go diff --git a/registry/client/auth/api_version.go b/registry/client/auth/api_version.go new file mode 100644 index 00000000..df095474 --- /dev/null +++ b/registry/client/auth/api_version.go @@ -0,0 +1,58 @@ +package auth + +import ( + "net/http" + "strings" +) + +// APIVersion represents a version of an API including its +// type and version number. +type APIVersion struct { + // Type refers to the name of a specific API specification + // such as "registry" + Type string + + // Version is the vesion of the API specification implemented, + // This may omit the revision number and only include + // the major and minor version, such as "2.0" + Version string +} + +// String returns the string formatted API Version +func (v APIVersion) String() string { + return v.Type + "/" + v.Version +} + +// APIVersions gets the API versions out of an HTTP response using the provided +// version header as the key for the HTTP header. +func APIVersions(resp *http.Response, versionHeader string) []APIVersion { + versions := []APIVersion{} + if versionHeader != "" { + for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey(versionHeader)] { + for _, version := range strings.Fields(supportedVersions) { + versions = append(versions, ParseAPIVersion(version)) + } + } + } + return versions +} + +// ParseAPIVersion parses an API version string into an APIVersion +// Format (Expected, not enforced): +// API version string = '/' +// API type = [a-z][a-z0-9]* +// API version = [0-9]+(\.[0-9]+)? +// TODO(dmcgowan): Enforce format, add error condition, remove unknown type +func ParseAPIVersion(versionStr string) APIVersion { + idx := strings.IndexRune(versionStr, '/') + if idx == -1 { + return APIVersion{ + Type: "unknown", + Version: versionStr, + } + } + return APIVersion{ + Type: strings.ToLower(versionStr[:idx]), + Version: versionStr[idx+1:], + } +} diff --git a/registry/client/auth/authchallenge.go b/registry/client/auth/authchallenge.go index e3abfb11..5d371646 100644 --- a/registry/client/auth/authchallenge.go +++ b/registry/client/auth/authchallenge.go @@ -1,14 +1,10 @@ package auth import ( - "fmt" "net/http" "strings" ) -// Octet types from RFC 2616. -type octetType byte - // Challenge carries information from a WWW-Authenticate response header. // See RFC 2617. type Challenge struct { @@ -19,6 +15,9 @@ type Challenge struct { Parameters map[string]string } +// Octet types from RFC 2616. +type octetType byte + var octetTypes [256]octetType const ( @@ -58,36 +57,17 @@ func init() { } } -// Ping pings the provided endpoint to determine its required authorization challenges. -// If a version header is provided, the versions will be returned. -func Ping(client *http.Client, endpoint, versionHeader string) ([]Challenge, []string, error) { - req, err := http.NewRequest("GET", endpoint, nil) - if err != nil { - return nil, nil, err - } - - resp, err := client.Do(req) - if err != nil { - return nil, nil, err - } - defer resp.Body.Close() - - versions := []string{} - if versionHeader != "" { - for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey(versionHeader)] { - versions = append(versions, strings.Fields(supportedVersions)...) - } - } - +// ResponseChallenges returns a list of authorization challenges +// for the given http Response. Challenges are only checked if +// the response status code was a 401. +func ResponseChallenges(resp *http.Response) []Challenge { if resp.StatusCode == http.StatusUnauthorized { // Parse the WWW-Authenticate Header and store the challenges // on this endpoint object. - return parseAuthHeader(resp.Header), versions, nil - } else if resp.StatusCode != http.StatusOK { - return nil, versions, fmt.Errorf("unable to get valid ping response: %d", resp.StatusCode) + return parseAuthHeader(resp.Header) } - return nil, versions, nil + return nil } func parseAuthHeader(header http.Header) []Challenge { diff --git a/registry/client/auth/session_test.go b/registry/client/auth/session_test.go index f16836da..3d19d4a7 100644 --- a/registry/client/auth/session_test.go +++ b/registry/client/auth/session_test.go @@ -42,8 +42,9 @@ func testServerWithAuth(rrm testutil.RequestResponseMap, authenticate string, au wrapper := &testAuthenticationWrapper{ headers: http.Header(map[string][]string{ - "Docker-Distribution-API-Version": {"registry/2.0"}, - "WWW-Authenticate": {authenticate}, + "X-API-Version": {"registry/2.0"}, + "X-Multi-API-Version": {"registry/2.0", "registry/2.1", "trust/1.0"}, + "WWW-Authenticate": {authenticate}, }), authCheck: authCheck, next: h, @@ -53,6 +54,18 @@ func testServerWithAuth(rrm testutil.RequestResponseMap, authenticate string, au return s.URL, s.Close } +// ping pings the provided endpoint to determine its required authorization challenges. +// If a version header is provided, the versions will be returned. +func ping(endpoint, versionHeader string) ([]Challenge, []APIVersion, error) { + resp, err := http.Get(endpoint) + if err != nil { + return nil, nil, err + } + defer resp.Body.Close() + + return ResponseChallenges(resp), APIVersions(resp, versionHeader), err +} + type testCredentialStore struct { username string password string @@ -112,10 +125,16 @@ func TestEndpointAuthorizeToken(t *testing.T) { e, c := testServerWithAuth(m, authenicate, validCheck) defer c() - challenges1, _, err := Ping(&http.Client{}, e+"/v2/", "") + challenges1, versions, err := ping(e+"/v2/", "x-api-version") if err != nil { t.Fatal(err) } + if len(versions) != 1 { + t.Fatalf("Unexpected version count: %d, expected 1", len(versions)) + } + if check := (APIVersion{Type: "registry", Version: "2.0"}); versions[0] != check { + t.Fatalf("Unexpected api version: %#v, expected %#v", versions[0], check) + } challengeMap1 := map[string][]Challenge{ e + "/v2/": challenges1, } @@ -138,10 +157,22 @@ func TestEndpointAuthorizeToken(t *testing.T) { e2, c2 := testServerWithAuth(m, authenicate, badCheck) defer c2() - challenges2, _, err := Ping(&http.Client{}, e+"/v2/", "") + challenges2, versions, err := ping(e+"/v2/", "x-multi-api-version") if err != nil { t.Fatal(err) } + if len(versions) != 3 { + t.Fatalf("Unexpected version count: %d, expected 3", len(versions)) + } + if check := (APIVersion{Type: "registry", Version: "2.0"}); versions[0] != check { + t.Fatalf("Unexpected api version: %#v, expected %#v", versions[0], check) + } + if check := (APIVersion{Type: "registry", Version: "2.1"}); versions[1] != check { + t.Fatalf("Unexpected api version: %#v, expected %#v", versions[1], check) + } + if check := (APIVersion{Type: "trust", Version: "1.0"}); versions[2] != check { + t.Fatalf("Unexpected api version: %#v, expected %#v", versions[2], check) + } challengeMap2 := map[string][]Challenge{ e + "/v2/": challenges2, } @@ -215,7 +246,7 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) { password: password, } - challenges, _, err := Ping(&http.Client{}, e+"/v2/", "") + challenges, _, err := ping(e+"/v2/", "") if err != nil { t.Fatal(err) } @@ -262,7 +293,7 @@ func TestEndpointAuthorizeBasic(t *testing.T) { password: password, } - challenges, _, err := Ping(&http.Client{}, e+"/v2/", "") + challenges, _, err := ping(e+"/v2/", "") if err != nil { t.Fatal(err) } From 3531b22b46b155c6ffb6a7eefe62032be4876421 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 30 Jun 2015 10:56:29 -0700 Subject: [PATCH 3/4] Add challenge manager interface Challenger manager interface is used to handle getting authorization challenges from an endpoint as well as extracting challenges from responses. Signed-off-by: Derek McGowan (github: dmcgowan) --- registry/client/auth/authchallenge.go | 53 +++++++++++++++++++++++++++ registry/client/auth/session.go | 28 +++++++------- registry/client/auth/session_test.go | 42 ++++++++++----------- 3 files changed, 87 insertions(+), 36 deletions(-) diff --git a/registry/client/auth/authchallenge.go b/registry/client/auth/authchallenge.go index 5d371646..a6ad45d8 100644 --- a/registry/client/auth/authchallenge.go +++ b/registry/client/auth/authchallenge.go @@ -1,7 +1,9 @@ package auth import ( + "fmt" "net/http" + "net/url" "strings" ) @@ -15,6 +17,57 @@ type Challenge struct { Parameters map[string]string } +// ChallengeManager manages the challenges for endpoints. +// The challenges are pulled out of HTTP responses. Only +// responses which expect challenges should be added to +// the manager, since a non-unauthorized request will be +// viewed as not requiring challenges. +type ChallengeManager interface { + // GetChallenges returns the challenges for the given + // endpoint URL. + GetChallenges(endpoint string) ([]Challenge, error) + + // AddResponse adds the response to the challenge + // manager. The challenges will be parsed out of + // the WWW-Authenicate headers and added to the + // URL which was produced the response. If the + // response was authorized, any challenges for the + // endpoint will be cleared. + AddResponse(resp *http.Response) error +} + +// NewSimpleChallengeManager returns an instance of +// ChallengeManger which only maps endpoints to challenges +// based on the responses which have been added the +// manager. The simple manager will make no attempt to +// perform requests on the endpoints or cache the responses +// to a backend. +func NewSimpleChallengeManager() ChallengeManager { + return simpleChallengeManager{} +} + +type simpleChallengeManager map[string][]Challenge + +func (m simpleChallengeManager) GetChallenges(endpoint string) ([]Challenge, error) { + challenges := m[endpoint] + return challenges, nil +} + +func (m simpleChallengeManager) AddResponse(resp *http.Response) error { + challenges := ResponseChallenges(resp) + if resp.Request == nil { + return fmt.Errorf("missing request reference") + } + urlCopy := url.URL{ + Path: resp.Request.URL.Path, + Host: resp.Request.URL.Host, + Scheme: resp.Request.URL.Scheme, + } + m[urlCopy.String()] = challenges + + return nil +} + // Octet types from RFC 2616. type octetType byte diff --git a/registry/client/auth/session.go b/registry/client/auth/session.go index 5512a9a1..27e1d9e3 100644 --- a/registry/client/auth/session.go +++ b/registry/client/auth/session.go @@ -36,15 +36,15 @@ type CredentialStore interface { // schemes. The handlers are tried in order, the higher priority authentication // methods should be first. The challengeMap holds a list of challenges for // a given root API endpoint (for example "https://registry-1.docker.io/v2/"). -func NewAuthorizer(challengeMap map[string][]Challenge, handlers ...AuthenticationHandler) transport.RequestModifier { +func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler) transport.RequestModifier { return &endpointAuthorizer{ - challenges: challengeMap, + challenges: manager, handlers: handlers, } } type endpointAuthorizer struct { - challenges map[string][]Challenge + challenges ChallengeManager handlers []AuthenticationHandler transport http.RoundTripper } @@ -63,18 +63,20 @@ func (ea *endpointAuthorizer) ModifyRequest(req *http.Request) error { pingEndpoint := ping.String() - challenges, ok := ea.challenges[pingEndpoint] - if !ok { - return nil + challenges, err := ea.challenges.GetChallenges(pingEndpoint) + if err != nil { + return err } - for _, handler := range ea.handlers { - for _, challenge := range challenges { - if challenge.Scheme != handler.Scheme() { - continue - } - if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil { - return err + if len(challenges) > 0 { + for _, handler := range ea.handlers { + for _, challenge := range challenges { + if challenge.Scheme != handler.Scheme() { + continue + } + if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil { + return err + } } } } diff --git a/registry/client/auth/session_test.go b/registry/client/auth/session_test.go index 3d19d4a7..1b4754ab 100644 --- a/registry/client/auth/session_test.go +++ b/registry/client/auth/session_test.go @@ -56,14 +56,18 @@ func testServerWithAuth(rrm testutil.RequestResponseMap, authenticate string, au // ping pings the provided endpoint to determine its required authorization challenges. // If a version header is provided, the versions will be returned. -func ping(endpoint, versionHeader string) ([]Challenge, []APIVersion, error) { +func ping(manager ChallengeManager, endpoint, versionHeader string) ([]APIVersion, error) { resp, err := http.Get(endpoint) if err != nil { - return nil, nil, err + return nil, err } defer resp.Body.Close() - return ResponseChallenges(resp), APIVersions(resp, versionHeader), err + if err := manager.AddResponse(resp); err != nil { + return nil, err + } + + return APIVersions(resp, versionHeader), err } type testCredentialStore struct { @@ -125,7 +129,8 @@ func TestEndpointAuthorizeToken(t *testing.T) { e, c := testServerWithAuth(m, authenicate, validCheck) defer c() - challenges1, versions, err := ping(e+"/v2/", "x-api-version") + challengeManager1 := NewSimpleChallengeManager() + versions, err := ping(challengeManager1, e+"/v2/", "x-api-version") if err != nil { t.Fatal(err) } @@ -135,10 +140,7 @@ func TestEndpointAuthorizeToken(t *testing.T) { if check := (APIVersion{Type: "registry", Version: "2.0"}); versions[0] != check { t.Fatalf("Unexpected api version: %#v, expected %#v", versions[0], check) } - challengeMap1 := map[string][]Challenge{ - e + "/v2/": challenges1, - } - transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap1, NewTokenHandler(nil, nil, repo1, "pull", "push"))) + transport1 := transport.NewTransport(nil, NewAuthorizer(challengeManager1, NewTokenHandler(nil, nil, repo1, "pull", "push"))) client := &http.Client{Transport: transport1} req, _ := http.NewRequest("GET", e+"/v2/hello", nil) @@ -157,7 +159,8 @@ func TestEndpointAuthorizeToken(t *testing.T) { e2, c2 := testServerWithAuth(m, authenicate, badCheck) defer c2() - challenges2, versions, err := ping(e+"/v2/", "x-multi-api-version") + challengeManager2 := NewSimpleChallengeManager() + versions, err = ping(challengeManager2, e+"/v2/", "x-multi-api-version") if err != nil { t.Fatal(err) } @@ -173,10 +176,7 @@ func TestEndpointAuthorizeToken(t *testing.T) { if check := (APIVersion{Type: "trust", Version: "1.0"}); versions[2] != check { t.Fatalf("Unexpected api version: %#v, expected %#v", versions[2], check) } - challengeMap2 := map[string][]Challenge{ - e + "/v2/": challenges2, - } - transport2 := transport.NewTransport(nil, NewAuthorizer(challengeMap2, NewTokenHandler(nil, nil, repo2, "pull", "push"))) + transport2 := transport.NewTransport(nil, NewAuthorizer(challengeManager2, NewTokenHandler(nil, nil, repo2, "pull", "push"))) client2 := &http.Client{Transport: transport2} req, _ = http.NewRequest("GET", e2+"/v2/hello", nil) @@ -246,14 +246,12 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) { password: password, } - challenges, _, err := ping(e+"/v2/", "") + challengeManager := NewSimpleChallengeManager() + _, err := ping(challengeManager, e+"/v2/", "") if err != nil { t.Fatal(err) } - challengeMap := map[string][]Challenge{ - e + "/v2/": challenges, - } - transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap, NewTokenHandler(nil, creds, repo, "pull", "push"), NewBasicHandler(creds))) + transport1 := transport.NewTransport(nil, NewAuthorizer(challengeManager, NewTokenHandler(nil, creds, repo, "pull", "push"), NewBasicHandler(creds))) client := &http.Client{Transport: transport1} req, _ := http.NewRequest("GET", e+"/v2/hello", nil) @@ -293,14 +291,12 @@ func TestEndpointAuthorizeBasic(t *testing.T) { password: password, } - challenges, _, err := ping(e+"/v2/", "") + challengeManager := NewSimpleChallengeManager() + _, err := ping(challengeManager, e+"/v2/", "") if err != nil { t.Fatal(err) } - challengeMap := map[string][]Challenge{ - e + "/v2/": challenges, - } - transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap, NewBasicHandler(creds))) + transport1 := transport.NewTransport(nil, NewAuthorizer(challengeManager, NewBasicHandler(creds))) client := &http.Client{Transport: transport1} req, _ := http.NewRequest("GET", e+"/v2/hello", nil) From 8fc782ae0976b0255e782845e5fc3b98be3b299c Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Wed, 8 Jul 2015 11:02:47 -0700 Subject: [PATCH 4/4] Fix typo in Version doc Signed-off-by: Derek McGowan (github: dmcgowan) --- registry/client/auth/api_version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/client/auth/api_version.go b/registry/client/auth/api_version.go index df095474..7d8f1d95 100644 --- a/registry/client/auth/api_version.go +++ b/registry/client/auth/api_version.go @@ -12,7 +12,7 @@ type APIVersion struct { // such as "registry" Type string - // Version is the vesion of the API specification implemented, + // Version is the version of the API specification implemented, // This may omit the revision number and only include // the major and minor version, such as "2.0" Version string