Skip to content

Commit

Permalink
Merge pull request #15945 from smarterclayton/ports
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue

Improve basic auth handling for canonical ports

Also make it easier to set custom scopes (used in another PR) for direct registry access.
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Sep 9, 2017
2 parents 2a02f4b + 22c9886 commit b902910
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 32 deletions.
42 changes: 38 additions & 4 deletions pkg/image/importer/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,54 @@ func NewContext(transport, insecureTransport http.RoundTripper) Context {
Transport: transport,
InsecureTransport: insecureTransport,
Challenges: challenge.NewSimpleManager(),
Actions: []string{"pull"},
}
}

type Context struct {
Transport http.RoundTripper
InsecureTransport http.RoundTripper
Challenges challenge.Manager
Scopes []auth.Scope
Actions []string
}

func (c Context) WithScopes(scopes ...auth.Scope) Context {
c.Scopes = scopes
return c
}

func (c Context) WithActions(actions ...string) Context {
c.Actions = actions
return c
}

func (c Context) WithCredentials(credentials auth.CredentialStore) RepositoryRetriever {
return c.WithAuthHandlers(func(rt http.RoundTripper, _ *url.URL, repoName string) []auth.AuthenticationHandler {
scopes := make([]auth.Scope, 0, 1+len(c.Scopes))
scopes = append(scopes, c.Scopes...)
if len(c.Actions) == 0 {
scopes = append(scopes, auth.RepositoryScope{Repository: repoName, Actions: []string{"pull"}})
} else {
scopes = append(scopes, auth.RepositoryScope{Repository: repoName, Actions: c.Actions})
}
return []auth.AuthenticationHandler{
auth.NewTokenHandlerWithOptions(auth.TokenHandlerOptions{
Transport: rt,
Credentials: credentials,
Scopes: scopes,
}),
auth.NewBasicHandler(credentials),
}
})
}

type AuthHandlersFunc func(transport http.RoundTripper, registry *url.URL, repoName string) []auth.AuthenticationHandler

func (c Context) WithAuthHandlers(fn AuthHandlersFunc) RepositoryRetriever {
return &repositoryRetriever{
context: c,
credentials: credentials,
credentials: fn,

pings: make(map[url.URL]error),
redirect: make(map[url.URL]*url.URL),
Expand All @@ -68,7 +103,7 @@ func (c Context) WithCredentials(credentials auth.CredentialStore) RepositoryRet

type repositoryRetriever struct {
context Context
credentials auth.CredentialStore
credentials AuthHandlersFunc

pings map[url.URL]error
redirect map[url.URL]*url.URL
Expand Down Expand Up @@ -111,8 +146,7 @@ func (r *repositoryRetriever) Repository(ctx gocontext.Context, registry *url.UR
// TODO: make multiple attempts if the first credential fails
auth.NewAuthorizer(
r.context.Challenges,
auth.NewTokenHandler(t, r.credentials, repoName, "pull"),
auth.NewBasicHandler(r.credentials),
r.credentials(t, registry, repoName)...,
),
)

Expand Down
85 changes: 59 additions & 26 deletions pkg/image/importer/credentials.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,31 @@ var (
emptyKeyring = &credentialprovider.BasicDockerKeyring{}
)

type refreshTokenKey struct {
url string
service string
}

type refreshTokenStore struct {
lock sync.Mutex
store map[refreshTokenKey]string
}

func (s *refreshTokenStore) RefreshToken(url *url.URL, service string) string {
s.lock.Lock()
defer s.lock.Unlock()
return s.store[refreshTokenKey{url: url.String(), service: service}]
}

func (s *refreshTokenStore) SetRefreshToken(url *url.URL, service string, token string) {
s.lock.Lock()
defer s.lock.Unlock()
if s.store == nil {
s.store = make(map[refreshTokenKey]string)
}
s.store[refreshTokenKey{url: url.String(), service: service}] = token
}

type noopCredentialStore struct{}

func (s *noopCredentialStore) Basic(url *url.URL) (string, string) {
Expand All @@ -36,7 +61,7 @@ func (s *noopCredentialStore) SetRefreshToken(url *url.URL, service string, toke
}

func NewBasicCredentials() *BasicCredentials {
return &BasicCredentials{}
return &BasicCredentials{refreshTokenStore: &refreshTokenStore{}}
}

type basicForURL struct {
Expand All @@ -46,6 +71,7 @@ type basicForURL struct {

type BasicCredentials struct {
creds []basicForURL
*refreshTokenStore
}

func (c *BasicCredentials) Add(url *url.URL, username, password string) {
Expand All @@ -65,38 +91,34 @@ func (c *BasicCredentials) Basic(url *url.URL) (string, string) {
return "", ""
}

func (c *BasicCredentials) RefreshToken(url *url.URL, service string) string {
return ""
}

func (c *BasicCredentials) SetRefreshToken(url *url.URL, service string, token string) {
}

func NewLocalCredentials() auth.CredentialStore {
return &keyringCredentialStore{credentialprovider.NewDockerKeyring()}
return &keyringCredentialStore{
DockerKeyring: credentialprovider.NewDockerKeyring(),
refreshTokenStore: &refreshTokenStore{},
}
}

type keyringCredentialStore struct {
credentialprovider.DockerKeyring
*refreshTokenStore
}

func (s *keyringCredentialStore) Basic(url *url.URL) (string, string) {
return basicCredentialsFromKeyring(s.DockerKeyring, url)
}

func (s *keyringCredentialStore) RefreshToken(url *url.URL, service string) string {
return ""
}

func (s *keyringCredentialStore) SetRefreshToken(url *url.URL, service string, token string) {
}

func NewCredentialsForSecrets(secrets []kapiv1.Secret) *SecretCredentialStore {
return &SecretCredentialStore{secrets: secrets}
return &SecretCredentialStore{
secrets: secrets,
refreshTokenStore: &refreshTokenStore{},
}
}

func NewLazyCredentialsForSecrets(secretsFn func() ([]kapiv1.Secret, error)) *SecretCredentialStore {
return &SecretCredentialStore{secretsFn: secretsFn}
return &SecretCredentialStore{
secretsFn: secretsFn,
refreshTokenStore: &refreshTokenStore{},
}
}

type SecretCredentialStore struct {
Expand All @@ -105,19 +127,14 @@ type SecretCredentialStore struct {
secretsFn func() ([]kapiv1.Secret, error)
err error
keyring credentialprovider.DockerKeyring

*refreshTokenStore
}

func (s *SecretCredentialStore) Basic(url *url.URL) (string, string) {
return basicCredentialsFromKeyring(s.init(), url)
}

func (s *SecretCredentialStore) RefreshToken(url *url.URL, service string) string {
return ""
}

func (s *SecretCredentialStore) SetRefreshToken(url *url.URL, service string, token string) {
}

func (s *SecretCredentialStore) Err() error {
s.lock.Lock()
defer s.lock.Unlock()
Expand Down Expand Up @@ -151,7 +168,13 @@ func (s *SecretCredentialStore) init() credentialprovider.DockerKeyring {

func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, target *url.URL) (string, string) {
// TODO: compare this logic to Docker authConfig in v2 configuration
value := target.Host + target.Path
var value string
if len(target.Scheme) == 0 || target.Scheme == "https" {
value = target.Host + target.Path
} else {
// only lookup credential for http that say they are for http
value = target.String()
}

// Lookup(...) expects an image (not a URL path).
// The keyring strips /v1/ and /v2/ version prefixes,
Expand All @@ -174,6 +197,16 @@ func basicCredentialsFromKeyring(keyring credentialprovider.DockerKeyring, targe
glog.V(5).Infof("Being asked for %s, trying %s for legacy behavior", target, "docker.io")
return basicCredentialsFromKeyring(keyring, &url.URL{Host: "docker.io"})
}

// try removing the canonical ports for the given requests
if (strings.HasSuffix(target.Host, ":443") && target.Scheme == "https") ||
(strings.HasSuffix(target.Host, ":80") && target.Scheme == "http") {
host := strings.SplitN(target.Host, ":", 2)[0]
glog.V(5).Infof("Being asked for %s, trying %s without port", target, host)

return basicCredentialsFromKeyring(keyring, &url.URL{Scheme: target.Scheme, Host: host, Path: target.Path})
}

glog.V(5).Infof("Unable to find a secret to match %s (%s)", target, value)
return "", ""
}
Expand Down
57 changes: 55 additions & 2 deletions pkg/image/importer/credentials_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
kapiv1 "k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/credentialprovider"

"github.com/golang/glog"
_ "github.com/openshift/origin/pkg/api/install"
)

Expand All @@ -29,7 +28,7 @@ func TestCredentialsForSecrets(t *testing.T) {
for i, secret := range secrets.Items {
err := kapiv1.Convert_api_Secret_To_v1_Secret(&secret, &secretsv1[i], nil)
if err != nil {
glog.V(2).Infof("Unable to make the Docker keyring for %s/%s secret: %v", secret.Name, secret.Namespace, err)
t.Logf("Unable to make the Docker keyring for %s/%s secret: %v", secret.Name, secret.Namespace, err)
continue
}
}
Expand Down Expand Up @@ -70,3 +69,57 @@ func TestBasicCredentials(t *testing.T) {
t.Fatalf("unexpected response: %s %s", u, p)
}
}

func Test_basicCredentialsFromKeyring(t *testing.T) {
fn := func(host string, entry credentialprovider.DockerConfigEntry) credentialprovider.DockerKeyring {
k := &credentialprovider.BasicDockerKeyring{}
k.Add(map[string]credentialprovider.DockerConfigEntry{host: entry})
return k
}
def := credentialprovider.DockerConfigEntry{
Username: "local_user",
Password: "local_pass",
}
type args struct {
keyring credentialprovider.DockerKeyring
target *url.URL
}
tests := []struct {
name string
args args
user string
password string
}{
{name: "exact", args: args{keyring: fn("localhost", def), target: &url.URL{Host: "localhost"}}, user: def.Username, password: def.Password},
{name: "https scheme", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: def.Username, password: def.Password},
{name: "canonical https", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "https", Host: "localhost:443"}}, user: def.Username, password: def.Password},
{name: "only https", args: args{keyring: fn("https://localhost", def), target: &url.URL{Host: "localhost"}}, user: def.Username, password: def.Password},
{name: "only https scheme", args: args{keyring: fn("https://localhost", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: def.Username, password: def.Password},
{name: "mismatched scheme - http", args: args{keyring: fn("http://localhost", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: def.Username, password: def.Password},

// this is not allowed by the credential keyring, but should be
{name: "exact http", args: args{keyring: fn("http://localhost", def), target: &url.URL{Scheme: "http", Host: "localhost:80"}}, user: "", password: ""},
{name: "keyring canonical https", args: args{keyring: fn("localhost:443", def), target: &url.URL{Scheme: "https", Host: "localhost"}}, user: "", password: ""},

// these should not be allowed
{name: "host is for port 80 only", args: args{keyring: fn("localhost:80", def), target: &url.URL{Host: "localhost"}}, user: "", password: ""},
{name: "host is for port 443 only", args: args{keyring: fn("localhost:443", def), target: &url.URL{Host: "localhost"}}, user: "", password: ""},
{name: "don't assume port 80 in keyring is https", args: args{keyring: fn("localhost:80", def), target: &url.URL{Scheme: "http", Host: "localhost"}}, user: "", password: ""},
{name: "canonical http", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "http", Host: "localhost:80"}}, user: "", password: ""},
{name: "http scheme", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "http", Host: "localhost"}}, user: "", password: ""},
{name: "https not canonical", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "https", Host: "localhost:80"}}, user: "", password: ""},
{name: "http not canonical", args: args{keyring: fn("localhost", def), target: &url.URL{Scheme: "http", Host: "localhost:443"}}, user: "", password: ""},
{name: "mismatched scheme", args: args{keyring: fn("https://localhost", def), target: &url.URL{Scheme: "http", Host: "localhost"}}, user: "", password: ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
user, password := basicCredentialsFromKeyring(tt.args.keyring, tt.args.target)
if user != tt.user {
t.Errorf("basicCredentialsFromKeyring() user = %v, actual = %v", user, tt.user)
}
if password != tt.password {
t.Errorf("basicCredentialsFromKeyring() password = %v, actual = %v", password, tt.password)
}
})
}
}

0 comments on commit b902910

Please sign in to comment.