From a1a73884f949da811f1e18ae08a6530b942d2f0c Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 7 Nov 2016 17:13:56 -0800 Subject: [PATCH] Refactor authorization challenges to its own package Split challenges into its own package. Avoids possible import cycle with challenges from client. Signed-off-by: Derek McGowan (github: dmcgowan) --- registry/client/auth/{ => challenge}/addr.go | 2 +- .../auth/{ => challenge}/authchallenge.go | 20 ++++++++-------- .../{ => challenge}/authchallenge_test.go | 6 ++--- registry/client/auth/session.go | 11 +++++---- registry/client/auth/session_test.go | 23 ++++++++++--------- registry/proxy/proxyauth.go | 5 ++-- registry/proxy/proxymanifeststore_test.go | 3 ++- registry/proxy/proxyregistry.go | 9 ++++---- 8 files changed, 42 insertions(+), 37 deletions(-) rename registry/client/auth/{ => challenge}/addr.go (97%) rename registry/client/auth/{ => challenge}/authchallenge.go (91%) rename registry/client/auth/{ => challenge}/authchallenge_test.go (97%) diff --git a/registry/client/auth/addr.go b/registry/client/auth/challenge/addr.go similarity index 97% rename from registry/client/auth/addr.go rename to registry/client/auth/challenge/addr.go index 6e777528..2c3ebe16 100644 --- a/registry/client/auth/addr.go +++ b/registry/client/auth/challenge/addr.go @@ -1,4 +1,4 @@ -package auth +package challenge import ( "net/url" diff --git a/registry/client/auth/authchallenge.go b/registry/client/auth/challenge/authchallenge.go similarity index 91% rename from registry/client/auth/authchallenge.go rename to registry/client/auth/challenge/authchallenge.go index 69d9d6fe..c9bdfc35 100644 --- a/registry/client/auth/authchallenge.go +++ b/registry/client/auth/challenge/authchallenge.go @@ -1,4 +1,4 @@ -package auth +package challenge import ( "fmt" @@ -18,12 +18,12 @@ type Challenge struct { Parameters map[string]string } -// ChallengeManager manages the challenges for endpoints. +// Manager 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 { +type Manager interface { // GetChallenges returns the challenges for the given // endpoint URL. GetChallenges(endpoint url.URL) ([]Challenge, error) @@ -37,19 +37,19 @@ type ChallengeManager interface { AddResponse(resp *http.Response) error } -// NewSimpleChallengeManager returns an instance of -// ChallengeManger which only maps endpoints to challenges +// NewSimpleManager returns an instance of +// Manger 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{ +func NewSimpleManager() Manager { + return &simpleManager{ Challanges: make(map[string][]Challenge), } } -type simpleChallengeManager struct { +type simpleManager struct { sync.RWMutex Challanges map[string][]Challenge } @@ -59,7 +59,7 @@ func normalizeURL(endpoint *url.URL) { endpoint.Host = canonicalAddr(endpoint) } -func (m *simpleChallengeManager) GetChallenges(endpoint url.URL) ([]Challenge, error) { +func (m *simpleManager) GetChallenges(endpoint url.URL) ([]Challenge, error) { normalizeURL(&endpoint) m.RLock() @@ -68,7 +68,7 @@ func (m *simpleChallengeManager) GetChallenges(endpoint url.URL) ([]Challenge, e return challenges, nil } -func (m *simpleChallengeManager) AddResponse(resp *http.Response) error { +func (m *simpleManager) AddResponse(resp *http.Response) error { challenges := ResponseChallenges(resp) if resp.Request == nil { return fmt.Errorf("missing request reference") diff --git a/registry/client/auth/authchallenge_test.go b/registry/client/auth/challenge/authchallenge_test.go similarity index 97% rename from registry/client/auth/authchallenge_test.go rename to registry/client/auth/challenge/authchallenge_test.go index 2716fba5..d4986b39 100644 --- a/registry/client/auth/authchallenge_test.go +++ b/registry/client/auth/challenge/authchallenge_test.go @@ -1,4 +1,4 @@ -package auth +package challenge import ( "fmt" @@ -50,7 +50,7 @@ func TestAuthChallengeNormalization(t *testing.T) { func testAuthChallengeNormalization(t *testing.T, host string) { - scm := NewSimpleChallengeManager() + scm := NewSimpleManager() url, err := url.Parse(fmt.Sprintf("http://%s/v2/", host)) if err != nil { @@ -86,7 +86,7 @@ func testAuthChallengeNormalization(t *testing.T, host string) { func testAuthChallengeConcurrent(t *testing.T, host string) { - scm := NewSimpleChallengeManager() + scm := NewSimpleManager() url, err := url.Parse(fmt.Sprintf("http://%s/v2/", host)) if err != nil { diff --git a/registry/client/auth/session.go b/registry/client/auth/session.go index d03d8ff0..ffc3384b 100644 --- a/registry/client/auth/session.go +++ b/registry/client/auth/session.go @@ -12,6 +12,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/distribution/registry/client" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" ) @@ -58,7 +59,7 @@ 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(manager ChallengeManager, handlers ...AuthenticationHandler) transport.RequestModifier { +func NewAuthorizer(manager challenge.Manager, handlers ...AuthenticationHandler) transport.RequestModifier { return &endpointAuthorizer{ challenges: manager, handlers: handlers, @@ -66,7 +67,7 @@ func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler) } type endpointAuthorizer struct { - challenges ChallengeManager + challenges challenge.Manager handlers []AuthenticationHandler transport http.RoundTripper } @@ -94,11 +95,11 @@ func (ea *endpointAuthorizer) ModifyRequest(req *http.Request) error { if len(challenges) > 0 { for _, handler := range ea.handlers { - for _, challenge := range challenges { - if challenge.Scheme != handler.Scheme() { + for _, c := range challenges { + if c.Scheme != handler.Scheme() { continue } - if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil { + if err := handler.AuthorizeRequest(req, c.Parameters); err != nil { return err } } diff --git a/registry/client/auth/session_test.go b/registry/client/auth/session_test.go index cfae4f97..4f54c75c 100644 --- a/registry/client/auth/session_test.go +++ b/registry/client/auth/session_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" "github.com/docker/distribution/testutil" ) @@ -65,7 +66,7 @@ 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(manager ChallengeManager, endpoint, versionHeader string) ([]APIVersion, error) { +func ping(manager challenge.Manager, endpoint, versionHeader string) ([]APIVersion, error) { resp, err := http.Get(endpoint) if err != nil { return nil, err @@ -149,7 +150,7 @@ func TestEndpointAuthorizeToken(t *testing.T) { e, c := testServerWithAuth(m, authenicate, validCheck) defer c() - challengeManager1 := NewSimpleChallengeManager() + challengeManager1 := challenge.NewSimpleManager() versions, err := ping(challengeManager1, e+"/v2/", "x-api-version") if err != nil { t.Fatal(err) @@ -176,7 +177,7 @@ func TestEndpointAuthorizeToken(t *testing.T) { e2, c2 := testServerWithAuth(m, authenicate, validCheck) defer c2() - challengeManager2 := NewSimpleChallengeManager() + challengeManager2 := challenge.NewSimpleManager() versions, err = ping(challengeManager2, e2+"/v2/", "x-multi-api-version") if err != nil { t.Fatal(err) @@ -273,7 +274,7 @@ func TestEndpointAuthorizeRefreshToken(t *testing.T) { e, c := testServerWithAuth(m, authenicate, validCheck) defer c() - challengeManager1 := NewSimpleChallengeManager() + challengeManager1 := challenge.NewSimpleManager() versions, err := ping(challengeManager1, e+"/v2/", "x-api-version") if err != nil { t.Fatal(err) @@ -306,7 +307,7 @@ func TestEndpointAuthorizeRefreshToken(t *testing.T) { e2, c2 := testServerWithAuth(m, authenicate, validCheck) defer c2() - challengeManager2 := NewSimpleChallengeManager() + challengeManager2 := challenge.NewSimpleManager() versions, err = ping(challengeManager2, e2+"/v2/", "x-api-version") if err != nil { t.Fatal(err) @@ -339,7 +340,7 @@ func TestEndpointAuthorizeRefreshToken(t *testing.T) { e3, c3 := testServerWithAuth(m, authenicate, validCheck) defer c3() - challengeManager3 := NewSimpleChallengeManager() + challengeManager3 := challenge.NewSimpleManager() versions, err = ping(challengeManager3, e3+"/v2/", "x-api-version") if err != nil { t.Fatal(err) @@ -401,7 +402,7 @@ func TestEndpointAuthorizeV2RefreshToken(t *testing.T) { e, c := testServerWithAuth(m, authenicate, validCheck) defer c() - challengeManager1 := NewSimpleChallengeManager() + challengeManager1 := challenge.NewSimpleManager() versions, err := ping(challengeManager1, e+"/v2/", "x-api-version") if err != nil { t.Fatal(err) @@ -496,7 +497,7 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) { password: password, } - challengeManager := NewSimpleChallengeManager() + challengeManager := challenge.NewSimpleManager() _, err := ping(challengeManager, e+"/v2/", "") if err != nil { t.Fatal(err) @@ -614,7 +615,7 @@ func TestEndpointAuthorizeTokenBasicWithExpiresIn(t *testing.T) { password: password, } - challengeManager := NewSimpleChallengeManager() + challengeManager := challenge.NewSimpleManager() _, err := ping(challengeManager, e+"/v2/", "") if err != nil { t.Fatal(err) @@ -765,7 +766,7 @@ func TestEndpointAuthorizeTokenBasicWithExpiresInAndIssuedAt(t *testing.T) { password: password, } - challengeManager := NewSimpleChallengeManager() + challengeManager := challenge.NewSimpleManager() _, err := ping(challengeManager, e+"/v2/", "") if err != nil { t.Fatal(err) @@ -845,7 +846,7 @@ func TestEndpointAuthorizeBasic(t *testing.T) { password: password, } - challengeManager := NewSimpleChallengeManager() + challengeManager := challenge.NewSimpleManager() _, err := ping(challengeManager, e+"/v2/", "") if err != nil { t.Fatal(err) diff --git a/registry/proxy/proxyauth.go b/registry/proxy/proxyauth.go index 6f4b37d5..7b405afc 100644 --- a/registry/proxy/proxyauth.go +++ b/registry/proxy/proxyauth.go @@ -7,6 +7,7 @@ import ( "github.com/docker/distribution/context" "github.com/docker/distribution/registry/client/auth" + "github.com/docker/distribution/registry/client/auth/challenge" ) const challengeHeader = "Docker-Distribution-Api-Version" @@ -62,7 +63,7 @@ func getAuthURLs(remoteURL string) ([]string, error) { } defer resp.Body.Close() - for _, c := range auth.ResponseChallenges(resp) { + for _, c := range challenge.ResponseChallenges(resp) { if strings.EqualFold(c.Scheme, "bearer") { authURLs = append(authURLs, c.Parameters["realm"]) } @@ -71,7 +72,7 @@ func getAuthURLs(remoteURL string) ([]string, error) { return authURLs, nil } -func ping(manager auth.ChallengeManager, endpoint, versionHeader string) error { +func ping(manager challenge.Manager, endpoint, versionHeader string) error { resp, err := http.Get(endpoint) if err != nil { return err diff --git a/registry/proxy/proxymanifeststore_test.go b/registry/proxy/proxymanifeststore_test.go index 26a26443..067e845a 100644 --- a/registry/proxy/proxymanifeststore_test.go +++ b/registry/proxy/proxymanifeststore_test.go @@ -12,6 +12,7 @@ import ( "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/client/auth" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/proxy/scheduler" "github.com/docker/distribution/registry/storage" "github.com/docker/distribution/registry/storage/cache/memory" @@ -77,7 +78,7 @@ func (m *mockChallenger) credentialStore() auth.CredentialStore { return nil } -func (m *mockChallenger) challengeManager() auth.ChallengeManager { +func (m *mockChallenger) challengeManager() challenge.Manager { return nil } diff --git a/registry/proxy/proxyregistry.go b/registry/proxy/proxyregistry.go index 9979866b..d64dcbb9 100644 --- a/registry/proxy/proxyregistry.go +++ b/registry/proxy/proxyregistry.go @@ -12,6 +12,7 @@ import ( "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth" + "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" "github.com/docker/distribution/registry/proxy/scheduler" "github.com/docker/distribution/registry/storage" @@ -102,7 +103,7 @@ func NewRegistryPullThroughCache(ctx context.Context, registry distribution.Name remoteURL: *remoteURL, authChallenger: &remoteAuthChallenger{ remoteURL: *remoteURL, - cm: auth.NewSimpleChallengeManager(), + cm: challenge.NewSimpleManager(), cs: cs, }, }, nil @@ -177,14 +178,14 @@ func (pr *proxyingRegistry) BlobStatter() distribution.BlobStatter { // authChallenger encapsulates a request to the upstream to establish credential challenges type authChallenger interface { tryEstablishChallenges(context.Context) error - challengeManager() auth.ChallengeManager + challengeManager() challenge.Manager credentialStore() auth.CredentialStore } type remoteAuthChallenger struct { remoteURL url.URL sync.Mutex - cm auth.ChallengeManager + cm challenge.Manager cs auth.CredentialStore } @@ -192,7 +193,7 @@ func (r *remoteAuthChallenger) credentialStore() auth.CredentialStore { return r.cs } -func (r *remoteAuthChallenger) challengeManager() auth.ChallengeManager { +func (r *remoteAuthChallenger) challengeManager() challenge.Manager { return r.cm }