diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver/resthandler.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver/resthandler.go index 4c1a3d76538d..87d937804435 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver/resthandler.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver/resthandler.go @@ -343,9 +343,7 @@ func DeleteResource(r RESTGracefulDeleter, checkBody bool, ctxFn ContextFunc, na return } ctx := ctxFn(req) - if len(namespace) > 0 { - ctx = api.WithNamespace(ctx, namespace) - } + ctx = api.WithNamespace(ctx, namespace) options := &api.DeleteOptions{} if checkBody { diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd/etcd.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd/etcd.go index b852fe159135..c277e7943bb6 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd/etcd.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd/etcd.go @@ -18,6 +18,7 @@ package etcd import ( "fmt" + "path" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" kubeerr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" @@ -111,6 +112,19 @@ func NamespaceKeyRootFunc(ctx api.Context, prefix string) string { return key } +// NoNamespaceKeyFunc is the default function for constructing etcd paths to a resource relative to prefix enforcing namespace rules. +// If a namespace is on context, it errors. +func NoNamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, error) { + ns, ok := api.NamespaceFrom(ctx) + if ok && len(ns) > 0 { + return "", kubeerr.NewBadRequest("Namespace parameter is not allowed.") + } + if len(name) == 0 { + return "", kubeerr.NewBadRequest("Name parameter required.") + } + return path.Join(prefix, name), nil +} + // NamespaceKeyFunc is the default function for constructing etcd paths to a resource relative to prefix enforcing namespace rules. // If no namespace is on context, it errors. func NamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, error) { diff --git a/examples/sample-app/README.md b/examples/sample-app/README.md index b8b2b325cd69..b98965bdabee 100644 --- a/examples/sample-app/README.md +++ b/examples/sample-app/README.md @@ -131,9 +131,9 @@ This section covers how to perform all the steps of building, deploying, and upd $ export CURL_CA_BUNDLE=`pwd`/openshift.local.certificates/ca/cert.crt $ sudo chmod +r "$KUBECONFIG" -4. Bind a user to the `view` role in the default namespace so you can observe progress in the web console (`anypassword` is an identity provider, `test-admin` is username) +4. Bind a user names `test-admin` to the `view` role in the default namespace so you can observe progress in the web console - $ openshift ex policy add-role-to-user view anypassword:test-admin + $ openshift ex policy add-role-to-user view test-admin 5. *Optional:* View the OpenShift web console in your browser by browsing to `https://:8443/console`. Login using the user `test-admin` and any password. @@ -186,7 +186,7 @@ This section covers how to perform all the steps of building, deploying, and upd 9. Create a new project in OpenShift. This creates a namespace `test` to contain the builds and app that we will generate below. - $ openshift ex new-project test --display-name="OpenShift 3 Sample" --description="This is an example project to demonstrate OpenShift v3" --admin=anypassword:test-admin + $ openshift ex new-project test --display-name="OpenShift 3 Sample" --description="This is an example project to demonstrate OpenShift v3" --admin=test-admin 10. *Optional:* View the OpenShift web console in your browser by browsing to `https://:8443/console`. Login using the user `test-admin` and any password. diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index c168323eaa81..9cee34a422e2 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -325,8 +325,8 @@ echo "ex policy: ok" # Test the commands the UI projects page tells users to run # These should match what is described in projects.html -osadm new-project ui-test-project --admin="anypassword:createuser" -osadm policy add-role-to-user admin anypassword:adduser -n ui-test-project +osadm new-project ui-test-project --admin="createuser" +osadm policy add-role-to-user admin adduser -n ui-test-project # Make sure project can be listed by osc (after auth cache syncs) sleep 2 && [ "$(osc get projects | grep 'ui-test-project')" ] # Make sure users got added @@ -335,10 +335,10 @@ sleep 2 && [ "$(osc get projects | grep 'ui-test-project')" ] echo "ui-project-commands: ok" # Test deleting and recreating a project -osadm new-project recreated-project --admin="anypassword:createuser1" +osadm new-project recreated-project --admin="createuser1" osc delete project recreated-project -osadm new-project recreated-project --admin="anypassword:createuser2" -osc describe policybinding master -n recreated-project | grep anypassword:createuser2 +osadm new-project recreated-project --admin="createuser2" +osc describe policybinding master -n recreated-project | grep createuser2 echo "ex new-project: ok" # Test running a router diff --git a/hack/test-end-to-end.sh b/hack/test-end-to-end.sh index aab17818815f..0443982f2405 100755 --- a/hack/test-end-to-end.sh +++ b/hack/test-end-to-end.sh @@ -215,10 +215,10 @@ wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1beta1/minions/127.0. export KUBERNETES_MASTER="${API_SCHEME}://${API_HOST}:${API_PORT}" # add e2e-user as a viewer for the default namespace so we can see infrastructure pieces appear -openshift ex policy add-role-to-user view anypassword:e2e-user --namespace=default +openshift ex policy add-role-to-user view e2e-user --namespace=default # create test project so that this shows up in the console -openshift ex new-project test --description="This is an example project to demonstrate OpenShift v3" --admin="anypassword:e2e-user" +openshift ex new-project test --description="This is an example project to demonstrate OpenShift v3" --admin="e2e-user" echo "The console should be available at ${API_SCHEME}://${PUBLIC_MASTER_HOST}:$(($API_PORT + 1)). You may need to visit ${API_SCHEME}://${PUBLIC_MASTER_HOST}:${API_PORT} first to accept the certificate." echo "Log in as 'e2e-user' to see the 'test' project." diff --git a/pkg/api/latest/latest.go b/pkg/api/latest/latest.go index d95982ddc566..6ccab1db0042 100644 --- a/pkg/api/latest/latest.go +++ b/pkg/api/latest/latest.go @@ -74,7 +74,7 @@ var originTypes = []string{ "Template", "TemplateConfig", "Route", "Project", - "User", "UserIdentityMapping", + "User", "Identity", "UserIdentityMapping", "OAuthClient", "OAuthClientAuthorization", "OAuthAccessToken", "OAuthAuthorizeToken", "Role", "RoleBinding", "Policy", "PolicyBinding", "ResourceAccessReview", "SubjectAccessReview", } diff --git a/pkg/auth/api/types.go b/pkg/auth/api/types.go index d7619245397d..af61d6fb32c5 100644 --- a/pkg/auth/api/types.go +++ b/pkg/auth/api/types.go @@ -4,13 +4,26 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" ) +const ( + // IdentityDisplayNameKey is the key for an optional display name in an identity's Extra map + IdentityDisplayNameKey = "name" + // IdentityEmailKey is the key for an optional email address in an identity's Extra map + IdentityEmailKey = "email" + // IdentityLoginKey is the key for an optional login in an identity's Extra map. + // This is useful when the immutable providerUserName is different than the login used to authenticate + // If present, this extra value is used as the preferred username + IdentityLoginKey = "login" +) + // UserIdentityInfo contains information about an identity. Identities are distinct from users. An authentication server of // some kind (like oauth for example) describes an identity. Our system controls the users mapped to this identity. type UserIdentityInfo interface { - // GetUserName uniquely identifies this particular identity for this provider. It is NOT guaranteed to be unique across providers - GetUserName() string + // GetIdentityName returns the name of this identity. It must be equal to GetProviderName() + ":" + GetProviderUserName() + GetIdentityName() string // GetProviderName returns the name of the provider of this identity. GetProviderName() string + // GetProviderUserName uniquely identifies this particular identity for this provider. It is NOT guaranteed to be unique across providers + GetProviderUserName() string // GetExtra is a map to allow providers to add additional fields that they understand GetExtra() map[string]string } @@ -37,27 +50,32 @@ type Grant struct { } type DefaultUserIdentityInfo struct { - UserName string - ProviderName string - Extra map[string]string + ProviderName string + ProviderUserName string + Extra map[string]string } -// NewDefaultUserIdentityInfo returns a DefaultUserIdentity info with a non-nil Extra component -func NewDefaultUserIdentityInfo(username string) DefaultUserIdentityInfo { - return DefaultUserIdentityInfo{ - UserName: username, - Extra: make(map[string]string), +// NewDefaultUserIdentityInfo returns a DefaultUserIdentityInfo with a non-nil Extra component +func NewDefaultUserIdentityInfo(providerName, providerUserName string) *DefaultUserIdentityInfo { + return &DefaultUserIdentityInfo{ + ProviderName: providerName, + ProviderUserName: providerUserName, + Extra: map[string]string{}, } } -func (i *DefaultUserIdentityInfo) GetUserName() string { - return i.UserName +func (i *DefaultUserIdentityInfo) GetIdentityName() string { + return i.ProviderName + ":" + i.ProviderUserName } func (i *DefaultUserIdentityInfo) GetProviderName() string { return i.ProviderName } +func (i *DefaultUserIdentityInfo) GetProviderUserName() string { + return i.ProviderUserName +} + func (i *DefaultUserIdentityInfo) GetExtra() map[string]string { return i.Extra } diff --git a/pkg/auth/authenticator/password/allowanypassword/anyauthpassword.go b/pkg/auth/authenticator/password/allowanypassword/anyauthpassword.go index 3a98c0eaea3d..f2d128f33cdb 100644 --- a/pkg/auth/authenticator/password/allowanypassword/anyauthpassword.go +++ b/pkg/auth/authenticator/password/allowanypassword/anyauthpassword.go @@ -12,12 +12,13 @@ import ( // alwaysAcceptPasswordAuthenticator approves any login attempt with non-blank username and password type alwaysAcceptPasswordAuthenticator struct { + providerName string identityMapper authapi.UserIdentityMapper } // New creates a new password authenticator that approves any login attempt with non-blank username and password -func New(identityMapper authapi.UserIdentityMapper) authenticator.Password { - return &alwaysAcceptPasswordAuthenticator{identityMapper} +func New(providerName string, identityMapper authapi.UserIdentityMapper) authenticator.Password { + return &alwaysAcceptPasswordAuthenticator{providerName, identityMapper} } // AuthenticatePassword approves any login attempt with non-blank username and password @@ -26,9 +27,7 @@ func (a alwaysAcceptPasswordAuthenticator) AuthenticatePassword(username, passwo return nil, false, nil } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: username, - } + identity := authapi.NewDefaultUserIdentityInfo(a.providerName, username) user, err := a.identityMapper.UserFor(identity) glog.V(4).Infof("Got userIdentityMapping: %#v", user) if err != nil { diff --git a/pkg/auth/authenticator/password/basicauthpassword/basicauthpassword.go b/pkg/auth/authenticator/password/basicauthpassword/basicauthpassword.go index cc2baa3fc7e7..af653285cad6 100644 --- a/pkg/auth/authenticator/password/basicauthpassword/basicauthpassword.go +++ b/pkg/auth/authenticator/password/basicauthpassword/basicauthpassword.go @@ -24,15 +24,22 @@ import ( // A successful response may also include name and/or email: // {"id":"userid", "name": "User Name", "email":"user@example.com"} type Authenticator struct { - url string - mapper authapi.UserIdentityMapper + providerName string + url string + mapper authapi.UserIdentityMapper } // RemoteUserData holds user data returned from a remote basic-auth protected endpoint. // These field names can not be changed unless external integrators are also updated. type RemoteUserData struct { - ID string - Name string + // ID is the immutable identity of the user + ID string + // Login is the optional login of the user + // Useful when the id is different than the username/login used by the user to authenticate + Login string + // Name is the optional display name of the user + Name string + // Email is the optional email address of the user Email string } @@ -42,8 +49,8 @@ type RemoteError struct { } // New returns an authenticator which will make a basic auth call to the given url. -func New(url string, mapper authapi.UserIdentityMapper) authenticator.Password { - return &Authenticator{url, mapper} +func New(providerName string, url string, mapper authapi.UserIdentityMapper) authenticator.Password { + return &Authenticator{providerName, url, mapper} } func (a *Authenticator) AuthenticatePassword(username, password string) (user.Info, bool, error) { @@ -89,13 +96,10 @@ func (a *Authenticator) AuthenticatePassword(username, password string) (user.In return nil, false, errors.New("Could not retrieve user data") } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: username, - Extra: map[string]string{ - "name": remoteUserData.Name, - "email": remoteUserData.Email, - }, - } + identity := authapi.NewDefaultUserIdentityInfo(a.providerName, remoteUserData.ID) + identity.Extra[authapi.IdentityDisplayNameKey] = remoteUserData.Name + identity.Extra[authapi.IdentityLoginKey] = remoteUserData.Login + identity.Extra[authapi.IdentityEmailKey] = remoteUserData.Email user, err := a.mapper.UserFor(identity) glog.V(4).Infof("Got userIdentityMapping: %#v", user) if err != nil { diff --git a/pkg/auth/authenticator/password/htpasswd/htpasswd.go b/pkg/auth/authenticator/password/htpasswd/htpasswd.go index 4f269551399c..b4fae9398c4b 100644 --- a/pkg/auth/authenticator/password/htpasswd/htpasswd.go +++ b/pkg/auth/authenticator/password/htpasswd/htpasswd.go @@ -17,17 +17,19 @@ import ( // Authenticator watches a file generated by htpasswd to validate usernames and passwords type Authenticator struct { - file string - fileInfo os.FileInfo - mapper authapi.UserIdentityMapper - usernames map[string]string + providerName string + file string + fileInfo os.FileInfo + mapper authapi.UserIdentityMapper + usernames map[string]string } // New returns an authenticator which will validate usernames and passwords against the given htpasswd file -func New(file string, mapper authapi.UserIdentityMapper) (authenticator.Password, error) { +func New(providerName string, file string, mapper authapi.UserIdentityMapper) (authenticator.Password, error) { auth := &Authenticator{ - file: file, - mapper: mapper, + providerName: providerName, + file: file, + mapper: mapper, } if err := auth.loadIfNeeded(); err != nil { return nil, err @@ -52,9 +54,7 @@ func (a *Authenticator) AuthenticatePassword(username, password string) (user.In return nil, false, err } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: username, - } + identity := authapi.NewDefaultUserIdentityInfo(a.providerName, username) user, err := a.mapper.UserFor(identity) glog.V(4).Infof("Got userIdentityMapping: %#v", user) if err != nil { diff --git a/pkg/auth/authenticator/password/oauthpassword/oauthpassword.go b/pkg/auth/authenticator/password/oauthpassword/oauthpassword.go index 570e53811143..cf7cb8a49056 100644 --- a/pkg/auth/authenticator/password/oauthpassword/oauthpassword.go +++ b/pkg/auth/authenticator/password/oauthpassword/oauthpassword.go @@ -11,12 +11,13 @@ import ( ) type Authenticator struct { - mapper authapi.UserIdentityMapper - client *osincli.Client + providerName string + mapper authapi.UserIdentityMapper + client *osincli.Client } -func New(client *osincli.Client, identityMapper authapi.UserIdentityMapper) authenticator.Password { - return &Authenticator{identityMapper, client} +func New(providerName string, client *osincli.Client, identityMapper authapi.UserIdentityMapper) authenticator.Password { + return &Authenticator{providerName, identityMapper, client} } func (a *Authenticator) AuthenticatePassword(username, password string) (user.Info, bool, error) { @@ -33,10 +34,8 @@ func (a *Authenticator) AuthenticatePassword(username, password string) (user.In return nil, false, err } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: username, - Extra: map[string]string{"token": token.AccessToken}, - } + identity := authapi.NewDefaultUserIdentityInfo(a.providerName, username) + identity.Extra["token"] = token.AccessToken user, err := a.mapper.UserFor(identity) glog.V(4).Infof("Got userIdentityMapping: %#v", user) if err != nil { diff --git a/pkg/auth/authenticator/request/headerrequest/requestheader.go b/pkg/auth/authenticator/request/headerrequest/requestheader.go index 47aff4437f69..a77e536ec0e7 100644 --- a/pkg/auth/authenticator/request/headerrequest/requestheader.go +++ b/pkg/auth/authenticator/request/headerrequest/requestheader.go @@ -22,12 +22,13 @@ func NewDefaultConfig() *Config { } type Authenticator struct { - config *Config - mapper authapi.UserIdentityMapper + providerName string + config *Config + mapper authapi.UserIdentityMapper } -func NewAuthenticator(config *Config, mapper authapi.UserIdentityMapper) *Authenticator { - return &Authenticator{config, mapper} +func NewAuthenticator(providerName string, config *Config, mapper authapi.UserIdentityMapper) *Authenticator { + return &Authenticator{providerName, config, mapper} } func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, error) { @@ -46,9 +47,7 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, return nil, false, nil } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: username, - } + identity := authapi.NewDefaultUserIdentityInfo(a.providerName, username) user, err := a.mapper.UserFor(identity) if err != nil { return nil, false, err diff --git a/pkg/auth/authenticator/request/headerrequest/requestheader_test.go b/pkg/auth/authenticator/request/headerrequest/requestheader_test.go index 5273ffe60ce5..f2c3a65d562a 100644 --- a/pkg/auth/authenticator/request/headerrequest/requestheader_test.go +++ b/pkg/auth/authenticator/request/headerrequest/requestheader_test.go @@ -11,7 +11,7 @@ import ( type TestUserIdentityMapper struct{} func (m *TestUserIdentityMapper) UserFor(identityInfo api.UserIdentityInfo) (user.Info, error) { - return &user.DefaultInfo{Name: identityInfo.GetUserName()}, nil + return &user.DefaultInfo{Name: identityInfo.GetProviderUserName()}, nil } func TestRequestHeader(t *testing.T) { @@ -61,7 +61,7 @@ func TestRequestHeader(t *testing.T) { for k, testcase := range testcases { mapper := &TestUserIdentityMapper{} - auth := NewAuthenticator(&Config{testcase.ConfiguredHeaders}, mapper) + auth := NewAuthenticator("testprovider", &Config{testcase.ConfiguredHeaders}, mapper) req := &http.Request{Header: testcase.RequestHeaders} user, ok, err := auth.AuthenticateRequest(req) diff --git a/pkg/auth/oauth/external/github/github.go b/pkg/auth/oauth/external/github/github.go index c05215e1797d..817dbfab52fa 100644 --- a/pkg/auth/oauth/external/github/github.go +++ b/pkg/auth/oauth/external/github/github.go @@ -22,7 +22,7 @@ const ( ) type provider struct { - clientID, clientSecret string + providerName, clientID, clientSecret string } type githubUser struct { @@ -32,8 +32,8 @@ type githubUser struct { Name string } -func NewProvider(clientID, clientSecret string) external.Provider { - return provider{clientID, clientSecret} +func NewProvider(providerName, clientID, clientSecret string) external.Provider { + return provider{providerName, clientID, clientSecret} } // NewConfig implements external/interfaces/Provider.NewConfig @@ -79,14 +79,10 @@ func (p provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentit return nil, false, errors.New("Could not retrieve GitHub id") } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: fmt.Sprintf("%d", userdata.ID), - Extra: map[string]string{ - "name": userdata.Name, - "login": userdata.Login, - "email": userdata.Email, - }, - } + identity := authapi.NewDefaultUserIdentityInfo(p.providerName, fmt.Sprintf("%d", userdata.ID)) + identity.Extra[authapi.IdentityDisplayNameKey] = userdata.Name + identity.Extra[authapi.IdentityLoginKey] = userdata.Login + identity.Extra[authapi.IdentityEmailKey] = userdata.Email glog.V(4).Infof("Got identity=%#v", identity) return identity, true, nil diff --git a/pkg/auth/oauth/external/github/github_test.go b/pkg/auth/oauth/external/github/github_test.go index 362e693182f7..4be57af8fa44 100644 --- a/pkg/auth/oauth/external/github/github_test.go +++ b/pkg/auth/oauth/external/github/github_test.go @@ -7,5 +7,5 @@ import ( ) func TestGithub(t *testing.T) { - _ = external.Provider(NewProvider("", "")) + _ = external.Provider(NewProvider("github", "clientid", "clientsecret")) } diff --git a/pkg/auth/oauth/external/google/google.go b/pkg/auth/oauth/external/google/google.go index 78bffa66ae5d..66e5f73d8e63 100644 --- a/pkg/auth/oauth/external/google/google.go +++ b/pkg/auth/oauth/external/google/google.go @@ -21,11 +21,11 @@ const ( ) type provider struct { - clientID, clientSecret string + providerName, clientID, clientSecret string } -func NewProvider(clientID, clientSecret string) external.Provider { - return provider{clientID, clientSecret} +func NewProvider(providerName, clientID, clientSecret string) external.Provider { + return provider{providerName, clientID, clientSecret} } // NewConfig implements external/interfaces/Provider.NewConfig @@ -60,19 +60,19 @@ func (p provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentit return nil, false, err } - id, _ := userdata["id"].(string) - email, _ := userdata["email"].(string) - if id == "" || email == "" { + subject, _ := userdata["sub"].(string) + if subject == "" { return nil, false, errors.New("Could not retrieve Google id") } - identity := &authapi.DefaultUserIdentityInfo{ - UserName: id, - Extra: map[string]string{ - "name": email, - "email": email, - }, + email, _ := userdata["email"].(string) + if email == "" { + return nil, false, errors.New("Could not retrieve Google email") } + + identity := authapi.NewDefaultUserIdentityInfo(p.providerName, subject) + identity.Extra[authapi.IdentityLoginKey] = email + identity.Extra[authapi.IdentityEmailKey] = email glog.V(4).Infof("identity=%v", identity) return identity, true, nil diff --git a/pkg/auth/oauth/external/google/google_test.go b/pkg/auth/oauth/external/google/google_test.go index 895bb4529a24..9e2541ebea3f 100644 --- a/pkg/auth/oauth/external/google/google_test.go +++ b/pkg/auth/oauth/external/google/google_test.go @@ -7,5 +7,5 @@ import ( ) func TestGoogle(t *testing.T) { - _ = external.Provider(NewProvider("", "")) + _ = external.Provider(NewProvider("google", "clientid", "clientsecret")) } diff --git a/pkg/auth/userregistry/identitymapper/identitymapper.go b/pkg/auth/userregistry/identitymapper/identitymapper.go deleted file mode 100644 index a8773ee872fa..000000000000 --- a/pkg/auth/userregistry/identitymapper/identitymapper.go +++ /dev/null @@ -1,38 +0,0 @@ -package identitymapper - -import ( - "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" - authapi "github.com/openshift/origin/pkg/auth/api" - userapi "github.com/openshift/origin/pkg/user/api" - uimap "github.com/openshift/origin/pkg/user/registry/useridentitymapping" -) - -type alwaysCreateUserIdentityToUserMapper struct { - providerID string - userIdentityRegistry uimap.Registry -} - -// NewAlwaysCreateUserIdentityToUserMapper always does a createOrUpdate for the passed identity -func NewAlwaysCreateUserIdentityToUserMapper(providerID string, userIdentityRegistry uimap.Registry) authapi.UserIdentityMapper { - return &alwaysCreateUserIdentityToUserMapper{providerID, userIdentityRegistry} -} - -// UserFor returns info about the user for whom identity info have been provided -func (p *alwaysCreateUserIdentityToUserMapper) UserFor(identityInfo authapi.UserIdentityInfo) (user.Info, error) { - userIdentityMapping := &userapi.UserIdentityMapping{ - Identity: userapi.Identity{ - Provider: p.providerID, // Provider id is imposed - UserName: identityInfo.GetUserName(), - Extra: identityInfo.GetExtra(), - }, - } - authoritativeMapping, _ /*created*/, err := p.userIdentityRegistry.CreateOrUpdateUserIdentityMapping(userIdentityMapping) - if err != nil { - return nil, err - } - - return &user.DefaultInfo{ - Name: authoritativeMapping.User.Name, - UID: string(authoritativeMapping.User.UID), - }, nil -} diff --git a/pkg/auth/userregistry/identitymapper/identitymapper_test.go b/pkg/auth/userregistry/identitymapper/identitymapper_test.go deleted file mode 100644 index d37781c8fce6..000000000000 --- a/pkg/auth/userregistry/identitymapper/identitymapper_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package identitymapper - -import ( - "testing" - - authapi "github.com/openshift/origin/pkg/auth/api" - "github.com/openshift/origin/pkg/user/registry/test" -) - -func TestProvisionUser(t *testing.T) { - userIdentityRegistry := &test.UserIdentityMappingRegistry{} - providerID := "papa" - identityMapper := NewAlwaysCreateUserIdentityToUserMapper(providerID, userIdentityRegistry) - identity := &authapi.DefaultUserIdentityInfo{ - UserName: "oscar", - } - - identityMapper.UserFor(identity) - if userIdentityRegistry.CreatedUserIdentityMapping.Identity.Provider != providerID { - t.Errorf("Expected provider to be set to %v, but it was actually %v", providerID, userIdentityRegistry.CreatedUserIdentityMapping.Identity.Provider) - } - if userIdentityRegistry.CreatedUserIdentityMapping.Identity.UserName != identity.GetUserName() { - t.Errorf("Expected username to be set to %v, but it was actually %v", identity.GetUserName(), userIdentityRegistry.CreatedUserIdentityMapping.Identity.UserName) - } - -} diff --git a/pkg/auth/userregistry/identitymapper/lookup.go b/pkg/auth/userregistry/identitymapper/lookup.go new file mode 100644 index 000000000000..a7f187b29039 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/lookup.go @@ -0,0 +1,31 @@ +package identitymapper + +import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kuser "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" + + authapi "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/user/registry/useridentitymapping" +) + +type lookupIdentityMapper struct { + registry useridentitymapping.Registry +} + +// NewLookupIdentityMapper returns a mapper that will look up existing mappings for identities +func NewLookupIdentityMapper(registry useridentitymapping.Registry) authapi.UserIdentityMapper { + return &lookupIdentityMapper{registry} +} + +// UserFor returns info about the user for whom identity info has been provided +func (p *lookupIdentityMapper) UserFor(info authapi.UserIdentityInfo) (kuser.Info, error) { + mapping, err := p.registry.GetUserIdentityMapping(kapi.NewContext(), info.GetIdentityName()) + if err != nil { + return nil, err + } + + return &kuser.DefaultInfo{ + Name: mapping.User.Name, + UID: string(mapping.User.UID), + }, nil +} diff --git a/pkg/auth/userregistry/identitymapper/provision.go b/pkg/auth/userregistry/identitymapper/provision.go new file mode 100644 index 000000000000..a01e6c6aa5bd --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/provision.go @@ -0,0 +1,191 @@ +package identitymapper + +import ( + "errors" + "fmt" + + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + kuser "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/golang/glog" + + authapi "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/user" + userapi "github.com/openshift/origin/pkg/user/api" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + userregistry "github.com/openshift/origin/pkg/user/registry/user" +) + +// UserNameGenerator returns a username +type UserNameGenerator func(base string, sequence int) string + +var ( + // MaxGenerateAttempts limits how many times we try to find an available username for a new identity + MaxGenerateAttempts = 100 + + // DefaultGenerator attempts to use the base name first, then "base2", "base3", ... + DefaultGenerator = UserNameGenerator(func(base string, sequence int) string { + if sequence == 0 { + return base + } + return fmt.Sprintf("%s%d", base, sequence+1) + }) +) + +type provisioningIdentityMapper struct { + identity identityregistry.Registry + user userregistry.Registry + generator UserNameGenerator + initializer user.Initializer +} + +// NewAlwaysCreateUserIdentityToUserMapper returns an IdentityMapper that does the following: +// 1. Returns an existing user if the identity exists and is associated with an existing user +// 2. Returns an error if the identity exists and is not associated with a user +// 3. Creates the identity and creates and returns a new user with a unique username if the identity does not yet exist +func NewAlwaysCreateUserIdentityToUserMapper(identityRegistry identityregistry.Registry, userRegistry userregistry.Registry) authapi.UserIdentityMapper { + return &provisioningIdentityMapper{identityRegistry, userRegistry, DefaultGenerator, user.NewDefaultUserInitStrategy()} +} + +// UserFor returns info about the user for whom identity info have been provided +func (p *provisioningIdentityMapper) UserFor(info authapi.UserIdentityInfo) (kuser.Info, error) { + // Retrying up to three times lets us handle race conditions with up to two conflicting identity providers without returning an error + // * A single race is possible on user creation for every conflicting identity provider + // * A single race is possible on user creation between two instances of the same provider + // * A single race is possible on identity creation between two instances of the same provider + // + // A race condition between three conflicting identity providers *and* multiple instances of the same identity provider + // seems like a reasonable situation to return an error (you would get an AlreadyExists error on either the user or the identity) + return p.userForWithRetries(info, 3) +} + +func (p *provisioningIdentityMapper) userForWithRetries(info authapi.UserIdentityInfo, allowedRetries int) (kuser.Info, error) { + ctx := kapi.NewContext() + + identity, err := p.identity.GetIdentity(ctx, info.GetIdentityName()) + + if kerrs.IsNotFound(err) { + user, err := p.createIdentityAndMapping(ctx, info) + // Only retry for AlreadyExists errors, which can occur in the following cases: + // * The same user was created by another identity provider with the same preferred username + // * The same user was created by another instance of this identity provider + // * The same identity was created by another instance of this identity provider + if kerrs.IsAlreadyExists(err) && allowedRetries > 0 { + return p.userForWithRetries(info, allowedRetries-1) + } + return user, err + } + + if err != nil { + return nil, err + } + + return p.getMapping(ctx, identity) +} + +// createIdentityAndMapping creates an identity with a valid user reference for the given identity info +func (p *provisioningIdentityMapper) createIdentityAndMapping(ctx kapi.Context, info authapi.UserIdentityInfo) (kuser.Info, error) { + // Build the part of the identity we know about + identity := &userapi.Identity{ + ObjectMeta: kapi.ObjectMeta{ + Name: info.GetIdentityName(), + }, + ProviderName: info.GetProviderName(), + ProviderUserName: info.GetProviderUserName(), + Extra: info.GetExtra(), + } + + // Get or create a persisted user pointing to the identity + persistedUser, err := p.getOrCreateUserForIdentity(ctx, identity) + if err != nil { + return nil, err + } + + // Create the identity pointing to the persistedUser + identity.User = kapi.ObjectReference{ + Name: persistedUser.Name, + UID: persistedUser.UID, + } + if _, err := p.identity.CreateIdentity(ctx, identity); err != nil { + return nil, err + } + + return &kuser.DefaultInfo{ + Name: persistedUser.Name, + UID: string(persistedUser.UID), + }, nil +} + +func (p *provisioningIdentityMapper) getOrCreateUserForIdentity(ctx kapi.Context, identity *userapi.Identity) (*userapi.User, error) { + + preferredUserName := getPreferredUserName(identity) + + // Iterate through the max allowed generated usernames + // If an existing user references this identity, associate the identity with that user and return + // Otherwise, create a user with the first generated user name that does not already exist and return. + // Names are created in a deterministic order, so the first one that isn't present gets created. + // In the case of a race, one will get to persist the user object and the other will fail. + for sequence := 0; sequence < MaxGenerateAttempts; sequence++ { + // Get the username we want + potentialUserName := p.generator(preferredUserName, sequence) + + // See if it already exists + persistedUser, err := p.user.GetUser(ctx, potentialUserName) + + if err != nil && !kerrs.IsNotFound(err) { + // Fail on errors other than "not found" + return nil, err + } + + if err != nil && kerrs.IsNotFound(err) { + // Try to create a user with the available name + desiredUser := &userapi.User{ + ObjectMeta: kapi.ObjectMeta{Name: potentialUserName}, + Identities: []string{identity.Name}, + } + + // Initialize from the identity + p.initializer.InitializeUser(identity, desiredUser) + + // Create the user + createdUser, err := p.user.CreateUser(ctx, desiredUser) + if err != nil { + return nil, err + } + return createdUser, nil + } + + if util.NewStringSet(persistedUser.Identities...).Has(identity.Name) { + // If the existing user references our identity, we're done + return persistedUser, nil + } + } + + return nil, errors.New("Could not create user, max attempts exceeded") +} + +func (p *provisioningIdentityMapper) getMapping(ctx kapi.Context, identity *userapi.Identity) (kuser.Info, error) { + if len(identity.User.Name) == 0 { + return nil, kerrs.NewNotFound("UserIdentityMapping", identity.Name) + } + u, err := p.user.GetUser(ctx, identity.User.Name) + if err != nil { + return nil, err + } + if u.UID != identity.User.UID { + glog.Errorf("identity.user.uid (%s) and user.uid (%s) do not match for identity %s", identity.User.UID, u.UID, identity.Name) + return nil, kerrs.NewNotFound("UserIdentityMapping", identity.Name) + } + return &kuser.DefaultInfo{ + Name: u.Name, + UID: string(u.UID), + }, nil +} + +func getPreferredUserName(identity *userapi.Identity) string { + if login, ok := identity.Extra[authapi.IdentityLoginKey]; ok && len(login) > 0 { + return login + } + return identity.ProviderUserName +} diff --git a/pkg/auth/userregistry/identitymapper/provision_test.go b/pkg/auth/userregistry/identitymapper/provision_test.go new file mode 100644 index 000000000000..6ed393468e15 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/provision_test.go @@ -0,0 +1,167 @@ +package identitymapper + +import ( + "reflect" + "testing" + + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/types" + + authapi "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" +) + +func TestGetPreferredUsername(t *testing.T) { + identity := &api.Identity{} + + identity.ProviderUserName = "foo" + if preferred := getPreferredUserName(identity); preferred != "foo" { + t.Errorf("Expected %s, got %s", "foo", preferred) + } + + identity.Extra = map[string]string{authapi.IdentityLoginKey: "bar"} + if preferred := getPreferredUserName(identity); preferred != "bar" { + t.Errorf("Expected %s, got %s", "bar", preferred) + } +} + +func TestProvisionNewIdentity(t *testing.T) { + expectedProviderName := "myprovidername" + expectedProviderUserName := "myusername" + expectedUserName := "myusername" + expectedUserUID := "myuseruid" + expectedIdentityName := "myprovidername:myusername" + + // Expect identity to fully specify a user name and uid + expectedCreateIdentity := &api.Identity{ + ObjectMeta: kapi.ObjectMeta{Name: expectedIdentityName}, + ProviderName: expectedProviderName, + ProviderUserName: expectedProviderUserName, + User: kapi.ObjectReference{ + Name: expectedUserName, + UID: types.UID(expectedUserUID), + }, + Extra: map[string]string{}, + } + // Expect user to be populated with the right name, display name, and identity + expectedCreateUser := &api.User{ + ObjectMeta: kapi.ObjectMeta{ + Name: expectedUserName, + }, + Identities: []string{expectedIdentityName}, + } + // Return a user containing a uid + expectedCreateUserResult := &api.User{ + ObjectMeta: kapi.ObjectMeta{ + Name: expectedUserName, + UID: types.UID(expectedUserUID), + }, + Identities: []string{expectedIdentityName}, + } + + expectedActions := []test.Action{ + {"GetIdentity", expectedIdentityName}, + {"GetUser", expectedProviderUserName}, + {"CreateUser", expectedCreateUser}, + {"CreateIdentity", expectedCreateIdentity}, + } + + actions := []test.Action{} + identityRegistry := &test.IdentityRegistry{ + Create: expectedCreateIdentity, + Actions: &actions, + } + userRegistry := &test.UserRegistry{ + Create: expectedCreateUserResult, + Actions: &actions, + } + + identityMapper := NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) + identity := authapi.NewDefaultUserIdentityInfo(expectedProviderName, expectedProviderUserName) + + identityMapper.UserFor(identity) + + for i, action := range actions { + if len(expectedActions) <= i { + t.Fatalf("Expected %d actions, got extras: %#v", len(expectedActions), actions[i:]) + } + expectedAction := expectedActions[i] + if !reflect.DeepEqual(expectedAction, action) { + t.Fatalf("Expected\n\t%s %#v\nGot\n\t%s %#v", expectedAction.Name, expectedAction.Object, action.Name, action.Object) + } + } +} + +func TestProvisionConflictingIdentity(t *testing.T) { + expectedProviderName := "myprovidername" + expectedProviderUserName := "myusername" + expectedIdentityName := "myprovidername:myusername" + expectedUserName := "myusername3" + expectedUserUID := "myuseruid" + + // Expect identity to fully specify a user name and uid + expectedCreateIdentity := &api.Identity{ + ObjectMeta: kapi.ObjectMeta{Name: expectedIdentityName}, + ProviderName: expectedProviderName, + ProviderUserName: expectedProviderUserName, + User: kapi.ObjectReference{ + Name: expectedUserName, + UID: types.UID(expectedUserUID), + }, + Extra: map[string]string{}, + } + // Expect user to be populated with the right name, display name, and identity + expectedCreateUser := &api.User{ + ObjectMeta: kapi.ObjectMeta{ + Name: expectedUserName, + }, + Identities: []string{expectedIdentityName}, + } + // Return a user containing a uid + expectedCreateUserResult := &api.User{ + ObjectMeta: kapi.ObjectMeta{ + Name: expectedUserName, + UID: types.UID(expectedUserUID), + }, + Identities: []string{expectedIdentityName}, + } + + expectedActions := []test.Action{ + {"GetIdentity", expectedIdentityName}, + {"GetUser", expectedProviderUserName}, + {"GetUser", expectedProviderUserName + "2"}, + {"GetUser", expectedProviderUserName + "3"}, + {"CreateUser", expectedCreateUser}, + {"CreateIdentity", expectedCreateIdentity}, + } + + actions := []test.Action{} + identityRegistry := &test.IdentityRegistry{ + Create: expectedCreateIdentity, + Actions: &actions, + } + userRegistry := &test.UserRegistry{ + Get: map[string]*api.User{ + expectedProviderUserName: {}, + expectedProviderUserName + "2": {}, + }, + Create: expectedCreateUserResult, + Actions: &actions, + } + + identityMapper := NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) + identity := authapi.NewDefaultUserIdentityInfo(expectedProviderName, expectedProviderUserName) + + identityMapper.UserFor(identity) + + for i, action := range actions { + if len(expectedActions) <= i { + t.Fatalf("Expected %d actions, got extras: %#v", len(expectedActions), actions[i:]) + } + expectedAction := expectedActions[i] + if !reflect.DeepEqual(expectedAction, action) { + t.Fatalf("Expected\n\t%s %#v\nGot\n\t%s %#v", expectedAction.Name, expectedAction.Object, action.Name, action.Object) + } + } +} diff --git a/pkg/authorization/api/types.go b/pkg/authorization/api/types.go index 83a4d8afae56..34f49e3df64c 100644 --- a/pkg/authorization/api/types.go +++ b/pkg/authorization/api/types.go @@ -52,7 +52,7 @@ var ( BuildGroupName: {"builds", "buildconfigs", "buildlogs"}, ImageGroupName: {"images", "imagerepositories", "imagerepositorymappings", "imagerepositorytags"}, DeploymentGroupName: {"deployments", "deploymentconfigs", "generatedeploymentconfigs", "deploymentconfigrollbacks"}, - UserGroupName: {"users", "useridentitymappings"}, + UserGroupName: {"identities", "users", "useridentitymappings"}, OAuthGroupName: {"oauthauthorizetokens", "oauthaccesstokens", "oauthclients", "oauthclientauthorizations"}, PolicyOwnerGroupName: {"policies", "policybindings"}, PermissionGrantingGroupName: {"roles", "rolebindings", "resourceaccessreviews", "subjectaccessreviews"}, diff --git a/pkg/client/client.go b/pkg/client/client.go index 80f9cd3f0bcc..a97ebd50eff9 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -26,6 +26,7 @@ type Interface interface { DeploymentsNamespacer DeploymentConfigsNamespacer RoutesNamespacer + IdentitiesInterface UsersInterface UserIdentityMappingsInterface ProjectsInterface @@ -99,6 +100,11 @@ func (c *Client) Users() UserInterface { return newUsers(c) } +// Identities provides a REST client for Identity +func (c *Client) Identities() IdentityInterface { + return newIdentities(c) +} + // UserIdentityMappings provides a REST client for UserIdentityMapping func (c *Client) UserIdentityMappings() UserIdentityMappingInterface { return newUserIdentityMappings(c) diff --git a/pkg/client/fake.go b/pkg/client/fake.go index 0a793d4029bd..40dda7385db7 100644 --- a/pkg/client/fake.go +++ b/pkg/client/fake.go @@ -62,6 +62,10 @@ func (c *Fake) Templates(namespace string) TemplateInterface { return &FakeTemplates{Fake: c} } +func (c *Fake) Identities() IdentityInterface { + return &FakeIdentities{Fake: c} +} + func (c *Fake) Users() UserInterface { return &FakeUsers{Fake: c} } diff --git a/pkg/client/fake_identities.go b/pkg/client/fake_identities.go new file mode 100644 index 000000000000..d1d96f9583f3 --- /dev/null +++ b/pkg/client/fake_identities.go @@ -0,0 +1,33 @@ +package client + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + userapi "github.com/openshift/origin/pkg/user/api" +) + +// FakeIdentities implements IdentitiesInterface. Meant to be embedded into a struct to get a default +// implementation. This makes faking out just the methods you want to test easier. +type FakeIdentities struct { + Fake *Fake +} + +func (c *FakeIdentities) List(label labels.Selector, field fields.Selector) (*userapi.IdentityList, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "list-identities"}) + return &userapi.IdentityList{}, nil +} + +func (c *FakeIdentities) Get(name string) (*userapi.Identity, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "get-identity", Value: name}) + return &userapi.Identity{}, nil +} + +func (c *FakeIdentities) Create(identity *userapi.Identity) (*userapi.Identity, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "create-identity", Value: identity}) + return &userapi.Identity{}, nil +} + +func (c *FakeIdentities) Update(identity *userapi.Identity) (*userapi.Identity, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "update-identity", Value: identity}) + return &userapi.Identity{}, nil +} diff --git a/pkg/client/fake_useridentitymappings.go b/pkg/client/fake_useridentitymappings.go index 6412609ef7ec..19c7cffb0380 100644 --- a/pkg/client/fake_useridentitymappings.go +++ b/pkg/client/fake_useridentitymappings.go @@ -10,7 +10,22 @@ type FakeUserIdentityMappings struct { Fake *Fake } -func (c *FakeUserIdentityMappings) CreateOrUpdate(mapping *userapi.UserIdentityMapping) (*userapi.UserIdentityMapping, bool, error) { - c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "createorupdate-useridentitymapping"}) - return nil, false, nil +func (c *FakeUserIdentityMappings) Get(name string) (*userapi.UserIdentityMapping, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "get-useridentitymapping", Value: name}) + return &userapi.UserIdentityMapping{}, nil +} + +func (c *FakeUserIdentityMappings) Create(mapping *userapi.UserIdentityMapping) (*userapi.UserIdentityMapping, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "create-useridentitymapping", Value: mapping}) + return &userapi.UserIdentityMapping{}, nil +} + +func (c *FakeUserIdentityMappings) Update(mapping *userapi.UserIdentityMapping) (*userapi.UserIdentityMapping, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "update-useridentitymapping", Value: mapping}) + return &userapi.UserIdentityMapping{}, nil +} + +func (c *FakeUserIdentityMappings) Delete(name string) error { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "delete-useridentitymapping", Value: name}) + return nil } diff --git a/pkg/client/fake_users.go b/pkg/client/fake_users.go index 01ceb1eef2c8..385348bc9465 100644 --- a/pkg/client/fake_users.go +++ b/pkg/client/fake_users.go @@ -1,6 +1,8 @@ package client import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" userapi "github.com/openshift/origin/pkg/user/api" ) @@ -10,7 +12,22 @@ type FakeUsers struct { Fake *Fake } +func (c *FakeUsers) List(label labels.Selector, field fields.Selector) (*userapi.UserList, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "list-users"}) + return &userapi.UserList{}, nil +} + func (c *FakeUsers) Get(name string) (*userapi.User, error) { c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "get-user", Value: name}) return &userapi.User{}, nil } + +func (c *FakeUsers) Create(user *userapi.User) (*userapi.User, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "create-user", Value: user}) + return &userapi.User{}, nil +} + +func (c *FakeUsers) Update(user *userapi.User) (*userapi.User, error) { + c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "update-user", Value: user}) + return &userapi.User{}, nil +} diff --git a/pkg/client/identities.go b/pkg/client/identities.go new file mode 100644 index 000000000000..ca07b23d996b --- /dev/null +++ b/pkg/client/identities.go @@ -0,0 +1,66 @@ +package client + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + userapi "github.com/openshift/origin/pkg/user/api" + _ "github.com/openshift/origin/pkg/user/api/v1beta1" +) + +// IdentitiesInterface has methods to work with Identity resources +type IdentitiesInterface interface { + Identities() IdentityInterface +} + +// IdentityInterface exposes methods on identity resources. +type IdentityInterface interface { + List(label labels.Selector, field fields.Selector) (*userapi.IdentityList, error) + Get(name string) (*userapi.Identity, error) + Create(identity *userapi.Identity) (*userapi.Identity, error) + Update(identity *userapi.Identity) (*userapi.Identity, error) +} + +// identities implements IdentityInterface interface +type identities struct { + r *Client +} + +// newIdentities returns an identities client +func newIdentities(c *Client) *identities { + return &identities{ + r: c, + } +} + +// List returns a list of identities that match the label and field selectors. +func (c *identities) List(label labels.Selector, field fields.Selector) (result *userapi.IdentityList, err error) { + result = &userapi.IdentityList{} + err = c.r.Get(). + Resource("identities"). + LabelsSelectorParam("labels", label). + FieldsSelectorParam("fields", field). + Do(). + Into(result) + return +} + +// Get returns information about a particular identity or an error +func (c *identities) Get(name string) (result *userapi.Identity, err error) { + result = &userapi.Identity{} + err = c.r.Get().Resource("identities").Name(name).Do().Into(result) + return +} + +// Create creates a new identity. Returns the server's representation of the identity and error if one occurs. +func (c *identities) Create(identity *userapi.Identity) (result *userapi.Identity, err error) { + result = &userapi.Identity{} + err = c.r.Post().Resource("identities").Body(identity).Do().Into(result) + return +} + +// Update updates the identity on server. Returns the server's representation of the identity and error if one occurs. +func (c *identities) Update(identity *userapi.Identity) (result *userapi.Identity, err error) { + result = &userapi.Identity{} + err = c.r.Put().Resource("identities").Name(identity.Name).Body(identity).Do().Into(result) + return +} diff --git a/pkg/client/useridentitymappings.go b/pkg/client/useridentitymappings.go index ea506a0947a5..8db56ad63b41 100644 --- a/pkg/client/useridentitymappings.go +++ b/pkg/client/useridentitymappings.go @@ -12,7 +12,10 @@ type UserIdentityMappingsInterface interface { // UserIdentityMappingInterface exposes methods on UserIdentityMapping resources. type UserIdentityMappingInterface interface { - CreateOrUpdate(*userapi.UserIdentityMapping) (*userapi.UserIdentityMapping, bool, error) + Get(string) (*userapi.UserIdentityMapping, error) + Create(*userapi.UserIdentityMapping) (*userapi.UserIdentityMapping, error) + Update(*userapi.UserIdentityMapping) (*userapi.UserIdentityMapping, error) + Delete(string) error } // userIdentityMappings implements UserIdentityMappingsNamespacer interface @@ -27,11 +30,29 @@ func newUserIdentityMappings(c *Client) *userIdentityMappings { } } -// CreateOrUpdate attempts to get or create a binding between a user and an identity. If user information -// is provided, the server will check whether it matches the expected value. At this time the server will only allow creation -// when the identity is new - future APIs may allow clients to bind additional identities to an account. -func (c *userIdentityMappings) CreateOrUpdate(mapping *userapi.UserIdentityMapping) (result *userapi.UserIdentityMapping, created bool, err error) { +// Get returns information about a particular mapping or an error +func (c *userIdentityMappings) Get(name string) (result *userapi.UserIdentityMapping, err error) { result = &userapi.UserIdentityMapping{} - err = c.r.Put().Resource("userIdentityMappings").Name(mapping.Name).Body(mapping).Do().WasCreated(&created).Into(result) + err = c.r.Get().Resource("userIdentityMappings").Name(name).Do().Into(result) + return +} + +// Create creates a new mapping. Returns the server's representation of the mapping and error if one occurs. +func (c *userIdentityMappings) Create(mapping *userapi.UserIdentityMapping) (result *userapi.UserIdentityMapping, err error) { + result = &userapi.UserIdentityMapping{} + err = c.r.Post().Resource("userIdentityMappings").Body(mapping).Do().Into(result) + return +} + +// Update updates the mapping on server. Returns the server's representation of the mapping and error if one occurs. +func (c *userIdentityMappings) Update(mapping *userapi.UserIdentityMapping) (result *userapi.UserIdentityMapping, err error) { + result = &userapi.UserIdentityMapping{} + err = c.r.Put().Resource("userIdentityMappings").Name(mapping.Name).Body(mapping).Do().Into(result) + return +} + +// Delete deletes the mapping on server. +func (c *userIdentityMappings) Delete(name string) (err error) { + err = c.r.Delete().Resource("userIdentityMappings").Name(name).Do().Error() return } diff --git a/pkg/client/users.go b/pkg/client/users.go index 16791f4097a1..30726a36464c 100644 --- a/pkg/client/users.go +++ b/pkg/client/users.go @@ -1,21 +1,26 @@ package client import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" userapi "github.com/openshift/origin/pkg/user/api" _ "github.com/openshift/origin/pkg/user/api/v1beta1" ) -// UsersInterface has methods to work with User resources in a namespace +// UsersInterface has methods to work with User resources type UsersInterface interface { Users() UserInterface } // UserInterface exposes methods on user resources. type UserInterface interface { + List(label labels.Selector, field fields.Selector) (*userapi.UserList, error) Get(name string) (*userapi.User, error) + Create(user *userapi.User) (*userapi.User, error) + Update(user *userapi.User) (*userapi.User, error) } -// users implements UserIdentityMappingsNamespacer interface +// users implements UserInterface interface type users struct { r *Client } @@ -27,9 +32,35 @@ func newUsers(c *Client) *users { } } +// List returns a list of users that match the label and field selectors. +func (c *users) List(label labels.Selector, field fields.Selector) (result *userapi.UserList, err error) { + result = &userapi.UserList{} + err = c.r.Get(). + Resource("users"). + LabelsSelectorParam("labels", label). + FieldsSelectorParam("fields", field). + Do(). + Into(result) + return +} + // Get returns information about a particular user or an error func (c *users) Get(name string) (result *userapi.User, err error) { result = &userapi.User{} err = c.r.Get().Resource("users").Name(name).Do().Into(result) return } + +// Create creates a new user. Returns the server's representation of the user and error if one occurs. +func (c *users) Create(user *userapi.User) (result *userapi.User, err error) { + result = &userapi.User{} + err = c.r.Post().Resource("users").Body(user).Do().Into(result) + return +} + +// Update updates the user on server. Returns the server's representation of the user and error if one occurs. +func (c *users) Update(user *userapi.User) (result *userapi.User, err error) { + result = &userapi.User{} + err = c.r.Put().Resource("users").Name(user.Name).Body(user).Do().Into(result) + return +} diff --git a/pkg/cmd/cli/describe/describer.go b/pkg/cmd/cli/describe/describer.go index fa908e0bc029..444b1cfeba99 100644 --- a/pkg/cmd/cli/describe/describer.go +++ b/pkg/cmd/cli/describe/describer.go @@ -8,6 +8,7 @@ import ( "text/tabwriter" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" kctl "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl" @@ -35,6 +36,8 @@ func DescriberFor(kind string, c *client.Client, kclient kclient.Interface, host return &DeploymentDescriber{c}, true case "DeploymentConfig": return NewDeploymentConfigDescriber(c, kclient), true + case "Identity": + return &IdentityDescriber{c}, true case "Image": return &ImageDescriber{c}, true case "ImageRepository": @@ -57,6 +60,10 @@ func DescriberFor(kind string, c *client.Client, kclient kclient.Interface, host return &RoleBindingDescriber{c}, true case "Role": return &RoleDescriber{c}, true + case "User": + return &UserDescriber{c}, true + case "UserIdentityMapping": + return &UserIdentityMappingDescriber{c}, true } return nil, false } @@ -609,3 +616,120 @@ func (d *TemplateDescriber) Describe(namespace, name string) (string, error) { return nil }) } + +// IdentityDescriber generates information about a user +type IdentityDescriber struct { + client.Interface +} + +func (d *IdentityDescriber) Describe(namespace, name string) (string, error) { + userClient := d.Users() + identityClient := d.Identities() + + identity, err := identityClient.Get(name) + if err != nil { + return "", err + } + + return tabbedString(func(out *tabwriter.Writer) error { + formatMeta(out, identity.ObjectMeta) + + if len(identity.User.Name) == 0 { + formatString(out, "User Name", identity.User.Name) + formatString(out, "User UID", identity.User.UID) + } else { + resolvedUser, err := userClient.Get(identity.User.Name) + + nameValue := identity.User.Name + uidValue := string(identity.User.UID) + + if kerrs.IsNotFound(err) { + nameValue += fmt.Sprintf(" (Error: User does not exist)") + } else if err != nil { + nameValue += fmt.Sprintf(" (Error: User lookup failed)") + } else { + if !util.NewStringSet(resolvedUser.Identities...).Has(name) { + nameValue += fmt.Sprintf(" (Error: User identities do not include %s)", name) + } + if resolvedUser.UID != identity.User.UID { + uidValue += fmt.Sprintf(" (Error: Actual user UID is %s)", string(resolvedUser.UID)) + } + } + + formatString(out, "User Name", nameValue) + formatString(out, "User UID", uidValue) + } + return nil + }) + +} + +// UserIdentityMappingDescriber generates information about a user +type UserIdentityMappingDescriber struct { + client.Interface +} + +func (d *UserIdentityMappingDescriber) Describe(namespace, name string) (string, error) { + c := d.UserIdentityMappings() + + mapping, err := c.Get(name) + if err != nil { + return "", err + } + + return tabbedString(func(out *tabwriter.Writer) error { + formatMeta(out, mapping.ObjectMeta) + formatString(out, "Identity", mapping.Identity.Name) + formatString(out, "User Name", mapping.User.Name) + formatString(out, "User UID", mapping.User.UID) + return nil + }) +} + +// UserDescriber generates information about a user +type UserDescriber struct { + client.Interface +} + +func (d *UserDescriber) Describe(namespace, name string) (string, error) { + userClient := d.Users() + identityClient := d.Identities() + + user, err := userClient.Get(name) + if err != nil { + return "", err + } + + return tabbedString(func(out *tabwriter.Writer) error { + formatMeta(out, user.ObjectMeta) + if len(user.FullName) > 0 { + formatString(out, "Full Name", user.FullName) + } + + if len(user.Identities) == 0 { + formatString(out, "Identities", "") + } else { + for i, identity := range user.Identities { + resolvedIdentity, err := identityClient.Get(identity) + + value := identity + if kerrs.IsNotFound(err) { + value += fmt.Sprintf(" (Error: Identity does not exist)") + } else if err != nil { + value += fmt.Sprintf(" (Error: Identity lookup failed)") + } else if resolvedIdentity.User.Name != name { + value += fmt.Sprintf(" (Error: Identity maps to user name '%s')", resolvedIdentity.User.Name) + } else if resolvedIdentity.User.UID != user.UID { + value += fmt.Sprintf(" (Error: Identity maps to user UID '%s')", resolvedIdentity.User.UID) + } + + if i == 0 { + formatString(out, "Identities", value) + } else { + fmt.Fprintf(out, " \t%s\n", value) + } + } + } + return nil + }) +} diff --git a/pkg/cmd/cli/describe/printer.go b/pkg/cmd/cli/describe/printer.go index adcb364fbd1c..a25fe04219d3 100644 --- a/pkg/cmd/cli/describe/printer.go +++ b/pkg/cmd/cli/describe/printer.go @@ -42,8 +42,9 @@ var ( oauthAccessTokenColumns = []string{"NAME", "USER NAME", "CLIENT NAME", "CREATED", "EXPIRES", "REDIRECT URI", "SCOPES"} oauthAuthorizeTokenColumns = []string{"NAME", "USER NAME", "CLIENT NAME", "CREATED", "EXPIRES", "REDIRECT URI", "SCOPES"} - userColumns = []string{"NAME", "UID", "FULL NAME"} - userIdentityMappingColumns = []string{"NAME", "IDENTITY PROVIDER", "IDENTITY USERNAME", "USER NAME"} + userColumns = []string{"NAME", "UID", "FULL NAME", "IDENTITIES"} + identityColumns = []string{"NAME", "IDP NAME", "IDP USER NAME", "USER NAME", "USER UID"} + userIdentityMappingColumns = []string{"NAME", "IDENTITY", "USER NAME", "USER UID"} ) func NewHumanReadablePrinter(noHeaders bool) *kctl.HumanReadablePrinter { @@ -85,6 +86,9 @@ func NewHumanReadablePrinter(noHeaders bool) *kctl.HumanReadablePrinter { p.Handler(oauthAuthorizeTokenColumns, printOAuthAuthorizeTokenList) p.Handler(userColumns, printUser) + p.Handler(userColumns, printUserList) + p.Handler(identityColumns, printIdentity) + p.Handler(identityColumns, printIdentityList) p.Handler(userIdentityMappingColumns, printUserIdentityMapping) return p } @@ -419,11 +423,32 @@ func printOAuthAuthorizeTokenList(list *oauthapi.OAuthAuthorizeTokenList, w io.W } func printUser(user *userapi.User, w io.Writer) error { - _, err := fmt.Fprintf(w, "%s\t%s\t%s\n", user.Name, user.UID, user.FullName) + _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", user.Name, user.UID, user.FullName, strings.Join(user.Identities, ", ")) return err } +func printUserList(list *userapi.UserList, w io.Writer) error { + for _, item := range list.Items { + if err := printUser(&item, w); err != nil { + return err + } + } + return nil +} + +func printIdentity(identity *userapi.Identity, w io.Writer) error { + _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", identity.Name, identity.ProviderName, identity.ProviderUserName, identity.User.Name, identity.User.UID) + return err +} +func printIdentityList(list *userapi.IdentityList, w io.Writer) error { + for _, item := range list.Items { + if err := printIdentity(&item, w); err != nil { + return err + } + } + return nil +} func printUserIdentityMapping(mapping *userapi.UserIdentityMapping, w io.Writer) error { - _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", mapping.Name, mapping.Identity.Provider, mapping.Identity.UserName, mapping.User.Name) + _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", mapping.Name, mapping.Identity.Name, mapping.User.Name, mapping.User.UID) return err } diff --git a/pkg/cmd/server/origin/auth.go b/pkg/cmd/server/origin/auth.go index b2faac8ecf6b..e6910bff17f2 100644 --- a/pkg/cmd/server/origin/auth.go +++ b/pkg/cmd/server/origin/auth.go @@ -51,8 +51,6 @@ import ( oauthetcd "github.com/openshift/origin/pkg/oauth/registry/etcd" "github.com/openshift/origin/pkg/oauth/server/osinserver" "github.com/openshift/origin/pkg/oauth/server/osinserver/registrystorage" - "github.com/openshift/origin/pkg/user" - useretcd "github.com/openshift/origin/pkg/user/registry/etcd" ) const ( @@ -378,17 +376,19 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand switch authHandlerType { case AuthHandlerGithub, AuthHandlerGoogle: callbackPath := path.Join(OpenShiftOAuthCallbackPrefix, string(authHandlerType)) - userRegistry := useretcd.New(c.EtcdHelper, user.NewDefaultUserInitStrategy()) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(string(authHandlerType) /*for now*/, userRegistry) + identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(c.IdentityRegistry, c.UserRegistry) var oauthProvider external.Provider if authHandlerType == AuthHandlerGoogle { - oauthProvider = google.NewProvider(c.GoogleClientID, c.GoogleClientSecret) + // TODO: use configured providerName + oauthProvider = google.NewProvider("google", c.GoogleClientID, c.GoogleClientSecret) } else if authHandlerType == AuthHandlerGithub { - oauthProvider = github.NewProvider(c.GithubClientID, c.GithubClientSecret) + // TODO: use configured providerName + oauthProvider = github.NewProvider("github", c.GithubClientID, c.GithubClientSecret) } state := external.DefaultState() + // TODO: use configured providerName oauthHandler, err := external.NewExternalOAuthRedirector(oauthProvider, state, c.MasterPublicAddr+callbackPath, successHandler, errorHandler, identityMapper) if err != nil { return nil, fmt.Errorf("unexpected error: %v", err) @@ -421,8 +421,7 @@ func (c *AuthConfig) getPasswordAuthenticator() (authenticator.Password, error) // TODO presumably we'll want either a list of what we've got or a way to describe a registry of these // hard-coded strings as a stand-in until it gets sorted out passwordAuthType := c.PasswordAuth - userRegistry := useretcd.New(c.EtcdHelper, user.NewDefaultUserInitStrategy()) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(string(passwordAuthType) /*for now*/, userRegistry) + identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(c.IdentityRegistry, c.UserRegistry) var passwordAuth authenticator.Password switch passwordAuthType { @@ -431,10 +430,12 @@ func (c *AuthConfig) getPasswordAuthenticator() (authenticator.Password, error) if len(basicAuthURL) == 0 { return nil, fmt.Errorf("BasicAuthURL is required to support basic password auth") } - passwordAuth = basicauthpassword.New(basicAuthURL, identityMapper) + // TODO: use configured providerName + passwordAuth = basicauthpassword.New("basicauthurl", basicAuthURL, identityMapper) case PasswordAuthAnyPassword: // Accepts any username and password - passwordAuth = allowanypassword.New(identityMapper) + // TODO: use configured providerName + passwordAuth = allowanypassword.New("anypassword", identityMapper) case PasswordAuthDeny: // Deny any username and password passwordAuth = denypassword.New() @@ -443,7 +444,8 @@ func (c *AuthConfig) getPasswordAuthenticator() (authenticator.Password, error) if len(htpasswdFile) == 0 { return nil, fmt.Errorf("HTPasswdFile is required to support htpasswd auth") } - if htpasswordAuth, err := htpasswd.New(htpasswdFile, identityMapper); err != nil { + // TODO: use configured providerName + if htpasswordAuth, err := htpasswd.New("htpasswd", htpasswdFile, identityMapper); err != nil { return nil, fmt.Errorf("Error loading htpasswd file %s: %v", htpasswdFile, err) } else { passwordAuth = htpasswordAuth @@ -500,12 +502,12 @@ func (c *AuthConfig) getAuthenticationRequestHandlerFromType(authRequestHandlerT return nil, fmt.Errorf("Unknown TokenStore %s. Must be oauth or file. The oauth server cannot start!", c.TokenStore) } case AuthRequestHandlerRequestHeader: - userRegistry := useretcd.New(c.EtcdHelper, user.NewDefaultUserInitStrategy()) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(string(authRequestHandlerType) /*for now*/, userRegistry) + identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(c.IdentityRegistry, c.UserRegistry) authRequestConfig := &headerrequest.Config{ UserNameHeaders: c.RequestHeaders, } - authRequestHandler = headerrequest.NewAuthenticator(authRequestConfig, identityMapper) + // TODO: use configured providerName + authRequestHandler = headerrequest.NewAuthenticator("requestheader", authRequestConfig, identityMapper) // Wrap with an x509 verifier if len(c.RequestHeaderCAFile) > 0 { diff --git a/pkg/cmd/server/origin/auth_config.go b/pkg/cmd/server/origin/auth_config.go index 935a8679b141..bcc40c917ae1 100644 --- a/pkg/cmd/server/origin/auth_config.go +++ b/pkg/cmd/server/origin/auth_config.go @@ -13,6 +13,10 @@ import ( configapi "github.com/openshift/origin/pkg/cmd/server/api" "github.com/openshift/origin/pkg/cmd/server/etcd" cmdutil "github.com/openshift/origin/pkg/cmd/util" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + identityetcd "github.com/openshift/origin/pkg/user/registry/identity/etcd" + userregistry "github.com/openshift/origin/pkg/user/registry/user" + useretcd "github.com/openshift/origin/pkg/user/registry/user/etcd" ) type AuthConfig struct { @@ -25,6 +29,9 @@ type AuthConfig struct { MasterRoots *x509.CertPool EtcdHelper tools.EtcdHelper + UserRegistry userregistry.Registry + IdentityRegistry identityregistry.Registry + // Max age of authorize tokens AuthorizeTokenMaxAgeSeconds int32 // Max age of access tokens @@ -103,6 +110,11 @@ func BuildAuthConfig(options configapi.MasterConfig) (*AuthConfig, error) { string(AuthRequestHandlerBasicAuth), }, ",") + userStorage := useretcd.NewREST(etcdHelper) + userRegistry := userregistry.NewRegistry(userStorage) + identityStorage := identityetcd.NewREST(etcdHelper) + identityRegistry := identityregistry.NewRegistry(identityStorage) + ret := &AuthConfig{ MasterAddr: options.OAuthConfig.MasterURL, MasterPublicAddr: options.OAuthConfig.MasterPublicURL, @@ -110,6 +122,9 @@ func BuildAuthConfig(options configapi.MasterConfig) (*AuthConfig, error) { MasterRoots: apiServerCAs, EtcdHelper: etcdHelper, + IdentityRegistry: identityRegistry, + UserRegistry: userRegistry, + // Max token ages AuthorizeTokenMaxAgeSeconds: cmdutil.EnvInt("OPENSHIFT_OAUTH_AUTHORIZE_TOKEN_MAX_AGE_SECONDS", 300, 1), AccessTokenMaxAgeSeconds: cmdutil.EnvInt("OPENSHIFT_OAUTH_ACCESS_TOKEN_MAX_AGE_SECONDS", 3600, 1), diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index 14f4cfae0a27..b1097dc291d9 100644 --- a/pkg/cmd/server/origin/master.go +++ b/pkg/cmd/server/origin/master.go @@ -78,9 +78,10 @@ import ( "github.com/openshift/origin/pkg/service" templateregistry "github.com/openshift/origin/pkg/template/registry" templateetcd "github.com/openshift/origin/pkg/template/registry/etcd" - "github.com/openshift/origin/pkg/user" - useretcd "github.com/openshift/origin/pkg/user/registry/etcd" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + identityetcd "github.com/openshift/origin/pkg/user/registry/identity/etcd" userregistry "github.com/openshift/origin/pkg/user/registry/user" + useretcd "github.com/openshift/origin/pkg/user/registry/user/etcd" "github.com/openshift/origin/pkg/user/registry/useridentitymapping" authorizationapi "github.com/openshift/origin/pkg/authorization/api" @@ -130,10 +131,15 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin buildEtcd := buildetcd.New(c.EtcdHelper) deployEtcd := deployetcd.New(c.EtcdHelper) routeEtcd := routeetcd.New(c.EtcdHelper) - userEtcd := useretcd.New(c.EtcdHelper, user.NewDefaultUserInitStrategy()) oauthEtcd := oauthetcd.New(c.EtcdHelper) authorizationEtcd := authorizationetcd.New(c.EtcdHelper) + userStorage := useretcd.NewREST(c.EtcdHelper) + userRegistry := userregistry.NewRegistry(userStorage) + identityStorage := identityetcd.NewREST(c.EtcdHelper) + identityRegistry := identityregistry.NewRegistry(identityStorage) + userIdentityMappingStorage := useridentitymapping.NewREST(userRegistry, identityRegistry) + imageStorage := imageetcd.NewREST(c.EtcdHelper) imageRegistry := image.NewRegistry(imageStorage) imageRepositoryStorage, imageRepositoryStatus := imagerepositoryetcd.NewREST(c.EtcdHelper, imagerepository.DefaultRegistryFunc(defaultRegistryFunc)) @@ -201,8 +207,9 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin "projects": projectregistry.NewREST(kclient.Namespaces(), c.ProjectAuthorizationCache), - "userIdentityMappings": useridentitymapping.NewREST(userEtcd), - "users": userregistry.NewREST(userEtcd), + "users": userStorage, + "identities": identityStorage, + "userIdentityMappings": userIdentityMappingStorage, "oAuthAuthorizeTokens": authorizetokenregistry.NewREST(oauthEtcd), "oAuthAccessTokens": accesstokenregistry.NewREST(oauthEtcd), diff --git a/pkg/user/api/register.go b/pkg/user/api/register.go index d342dbcda867..6bb3c07a077f 100644 --- a/pkg/user/api/register.go +++ b/pkg/user/api/register.go @@ -7,7 +7,9 @@ import ( func init() { api.Scheme.AddKnownTypes("", &User{}, + &UserList{}, &Identity{}, + &IdentityList{}, &UserIdentityMapping{}, ) } diff --git a/pkg/user/api/types.go b/pkg/user/api/types.go index 279edafaa0e7..5ad5660b536e 100644 --- a/pkg/user/api/types.go +++ b/pkg/user/api/types.go @@ -1,48 +1,58 @@ package api -import ( - kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" -) +import kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" // Auth system gets identity name and provider // POST to UserIdentityMapping, get back error or a filled out UserIdentityMapping object type User struct { - kapi.TypeMeta `json:",inline"` - kapi.ObjectMeta `json:"metadata,omitempty"` + kapi.TypeMeta + kapi.ObjectMeta - FullName string `json:"fullName,omitempty"` + FullName string + + Identities []string } type UserList struct { - kapi.TypeMeta `json:",inline"` - kapi.ObjectMeta `json:"metadata,omitempty"` - Items []User `json:"items"` + kapi.TypeMeta + kapi.ListMeta + Items []User } type Identity struct { - kapi.TypeMeta `json:",inline"` - kapi.ObjectMeta `json:"metadata,omitempty"` + kapi.TypeMeta + kapi.ObjectMeta + + // ProviderName is the source of identity information + ProviderName string - // Provider is the source of identity information - if empty, the default provider - // is assumed. - Provider string `json:"provider"` + // ProviderUserName uniquely represents this identity in the scope of the provider + ProviderUserName string - // UserName uniquely represents this identity in the scope of the identity provider - UserName string `json:"userName"` + // User is a reference to the user this identity is associated with + // Both Name and UID must be set + User kapi.ObjectReference + + Extra map[string]string +} - Extra map[string]string `json:"extra,omitempty"` +type IdentityList struct { + kapi.TypeMeta + kapi.ListMeta + Items []Identity } type UserIdentityMapping struct { - kapi.TypeMeta `json:",inline"` - kapi.ObjectMeta `json:"metadata,omitempty"` + kapi.TypeMeta + kapi.ObjectMeta - Identity Identity `json:"identity,omitempty"` - User User `json:"user,omitempty"` + Identity kapi.ObjectReference + User kapi.ObjectReference } func (*User) IsAnAPIObject() {} func (*UserList) IsAnAPIObject() {} func (*Identity) IsAnAPIObject() {} +func (*IdentityList) IsAnAPIObject() {} func (*UserIdentityMapping) IsAnAPIObject() {} diff --git a/pkg/user/api/v1beta1/register.go b/pkg/user/api/v1beta1/register.go index 6645e618d38e..cece522eb17b 100644 --- a/pkg/user/api/v1beta1/register.go +++ b/pkg/user/api/v1beta1/register.go @@ -7,7 +7,9 @@ import ( func init() { api.Scheme.AddKnownTypes("v1beta1", &User{}, + &UserList{}, &Identity{}, + &IdentityList{}, &UserIdentityMapping{}, ) } diff --git a/pkg/user/api/v1beta1/types.go b/pkg/user/api/v1beta1/types.go index 52fdba315d20..1a70db5c988e 100644 --- a/pkg/user/api/v1beta1/types.go +++ b/pkg/user/api/v1beta1/types.go @@ -1,8 +1,6 @@ package v1beta1 -import ( - kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" -) +import kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" // Auth system gets identity name and provider // POST to UserIdentityMapping, get back error or a filled out UserIdentityMapping object @@ -12,37 +10,49 @@ type User struct { kapi.ObjectMeta `json:"metadata,omitempty"` FullName string `json:"fullName,omitempty"` + + Identities []string `json:"identities"` } type UserList struct { - kapi.TypeMeta `json:",inline"` - kapi.ObjectMeta `json:"metadata,omitempty"` - Items []User `json:"items"` + kapi.TypeMeta `json:",inline"` + kapi.ListMeta `json:"metadata,omitempty"` + Items []User `json:"items"` } type Identity struct { kapi.TypeMeta `json:",inline"` kapi.ObjectMeta `json:"metadata,omitempty"` - // Provider is the source of identity information - if empty, the default provider - // is assumed. - Provider string `json:"provider"` + // ProviderName is the source of identity information + ProviderName string `json:"providerName"` - // UserName uniquely represents this identity in the scope of the identity provider - UserName string `json:"userName"` + // ProviderUserName uniquely represents this identity in the scope of the provider + ProviderUserName string `json:"providerUserName"` + + // User is a reference to the user this identity is associated with + // Both Name and UID must be set + User kapi.ObjectReference `json:"user"` Extra map[string]string `json:"extra,omitempty"` } +type IdentityList struct { + kapi.TypeMeta `json:",inline"` + kapi.ListMeta `json:"metadata,omitempty"` + Items []Identity `json:"items"` +} + type UserIdentityMapping struct { kapi.TypeMeta `json:",inline"` kapi.ObjectMeta `json:"metadata,omitempty"` - Identity Identity `json:"identity,omitempty"` - User User `json:"user,omitempty"` + Identity kapi.ObjectReference `json:"identity,omitempty"` + User kapi.ObjectReference `json:"user,omitempty"` } func (*User) IsAnAPIObject() {} func (*UserList) IsAnAPIObject() {} func (*Identity) IsAnAPIObject() {} +func (*IdentityList) IsAnAPIObject() {} func (*UserIdentityMapping) IsAnAPIObject() {} diff --git a/pkg/user/api/validation/validation.go b/pkg/user/api/validation/validation.go index 621d5863d8cb..1fb3daf50f36 100644 --- a/pkg/user/api/validation/validation.go +++ b/pkg/user/api/validation/validation.go @@ -1,6 +1,7 @@ package validation import ( + "fmt" "strings" errs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" @@ -8,37 +9,157 @@ import ( "github.com/openshift/origin/pkg/user/api" ) -func ValidateUserName(name string, prefix bool) (bool, string) { +func ValidateUserName(name string, _ bool) (bool, string) { if strings.Contains(name, "%") { - return false, `Usernames may not contain "%"` + return false, `may not contain "%"` } if strings.Contains(name, "/") { - return false, `Usernames may not contain "/"` + return false, `may not contain "/"` + } + if strings.Contains(name, ":") { + return false, `may not contain ":"` } if name == ".." { - return false, `Usernames may not equal ".."` + return false, `may not equal ".."` } if name == "." { - return false, `Usernames may not equal "."` + return false, `may not equal "."` + } + if name == "~" { + return false, `may not equal "~"` } return true, "" } +func ValidateIdentityName(name string, _ bool) (bool, string) { + if strings.Contains(name, "%") { + return false, `may not contain "%"` + } + if strings.Contains(name, "/") { + return false, `may not contain "/"` + } + if name == ".." { + return false, `may not equal ".."` + } + if name == "." { + return false, `may not equal "."` + } + parts := strings.Split(name, ":") + if len(parts) != 2 { + return false, `must be in the format :` + } + if len(parts[0]) == 0 { + return false, `must be in the format : with a non-empty providerName` + } + if len(parts[1]) == 0 { + return false, `must be in the format : with a non-empty providerUserName` + } + return true, "" +} + +func ValidateIdentityProviderName(name string) (bool, string) { + if strings.Contains(name, "%") { + return false, `may not contain "%"` + } + if strings.Contains(name, "/") { + return false, `may not contain "/"` + } + if strings.Contains(name, ":") { + return false, `may not contain ":"` + } + return true, "" +} + +func ValidateIdentityProviderUserName(name string) (bool, string) { + // Any provider user name must be a valid user name + return ValidateUserName(name, false) +} + func ValidateUser(user *api.User) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} allErrs = append(allErrs, kvalidation.ValidateObjectMeta(&user.ObjectMeta, false, ValidateUserName).Prefix("metadata")...) + for index, identity := range user.Identities { + if ok, msg := ValidateIdentityName(identity, false); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("identities[%d]", index), identity, msg)) + } + } + return allErrs +} + +func ValidateUserUpdate(user *api.User, old *api.User) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + allErrs = append(allErrs, kvalidation.ValidateObjectMetaUpdate(&old.ObjectMeta, &user.ObjectMeta).Prefix("metadata")...) return allErrs } func ValidateIdentity(identity *api.Identity) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} + allErrs = append(allErrs, kvalidation.ValidateObjectMeta(&identity.ObjectMeta, false, ValidateIdentityName).Prefix("metadata")...) + + if len(identity.ProviderName) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("providerName")) + } else if ok, msg := ValidateIdentityProviderName(identity.ProviderName); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("providerName", identity.ProviderName, msg)) + } + + if len(identity.ProviderUserName) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("providerUserName")) + } else if ok, msg := ValidateIdentityProviderName(identity.ProviderUserName); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("providerUserName", identity.ProviderUserName, msg)) + } + + if len(identity.ProviderName) > 0 && len(identity.ProviderUserName) > 0 { + expectedIdentityName := identity.ProviderName + ":" + identity.ProviderUserName + if identity.Name != expectedIdentityName { + allErrs = append(allErrs, errs.NewFieldInvalid("user.name", identity.User.Name, fmt.Sprintf("must be %s", expectedIdentityName))) + } + } + + if ok, msg := ValidateUserName(identity.User.Name, false); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("user.name", identity.User.Name, msg)) + } + if len(identity.User.Name) == 0 && len(identity.User.UID) != 0 { + allErrs = append(allErrs, errs.NewFieldInvalid("user.uid", identity.User.UID, "may not be set if user.name is empty")) + } + if len(identity.User.Name) != 0 && len(identity.User.UID) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("user.uid")) + } + return allErrs +} + +func ValidateIdentityUpdate(identity *api.Identity, old *api.Identity) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + + allErrs = append(allErrs, kvalidation.ValidateObjectMetaUpdate(&old.ObjectMeta, &identity.ObjectMeta).Prefix("metadata")...) + + if identity.ProviderName != old.ProviderName { + allErrs = append(allErrs, errs.NewFieldInvalid("providerName", identity.ProviderName, "may not change providerName")) + } + if identity.ProviderUserName != old.ProviderUserName { + allErrs = append(allErrs, errs.NewFieldInvalid("providerUserName", identity.ProviderUserName, "may not change providerUserName")) + } + return allErrs } func ValidateUserIdentityMapping(mapping *api.UserIdentityMapping) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, kvalidation.ValidateObjectMeta(&mapping.ObjectMeta, false, ValidateUserName).Prefix("metadata")...) - allErrs = append(allErrs, ValidateIdentity(&mapping.Identity).Prefix("identity")...) - allErrs = append(allErrs, ValidateUser(&mapping.User).Prefix("user")...) + allErrs = append(allErrs, kvalidation.ValidateObjectMeta(&mapping.ObjectMeta, false, ValidateIdentityName).Prefix("metadata")...) + if len(mapping.Identity.Name) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("identity.name")) + } + if mapping.Identity.Name != mapping.Name { + allErrs = append(allErrs, errs.NewFieldInvalid("identity.name", mapping.Identity.Name, "must match metadata.name")) + } + if len(mapping.User.Name) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("user.name")) + } + return allErrs +} + +func ValidateUserIdentityMappingUpdate(mapping *api.UserIdentityMapping, old *api.UserIdentityMapping) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + allErrs = append(allErrs, ValidateUserIdentityMapping(mapping)...) + allErrs = append(allErrs, kvalidation.ValidateObjectMetaUpdate(&old.ObjectMeta, &mapping.ObjectMeta).Prefix("metadata")...) return allErrs } diff --git a/pkg/user/api/validation/validation_test.go b/pkg/user/api/validation/validation_test.go new file mode 100644 index 000000000000..d7a38314b529 --- /dev/null +++ b/pkg/user/api/validation/validation_test.go @@ -0,0 +1,13 @@ +package validation + +import "testing" + +func TestValidateUser(t *testing.T) {} + +func TestValidateUserUpdate(t *testing.T) {} + +func TestValidateIdentity(t *testing.T) {} + +func TestValidateIdentityUpdate(t *testing.T) {} + +func TestValidateUserIdentityMapping(t *testing.T) {} diff --git a/pkg/user/initializer.go b/pkg/user/initializer.go index 35d98e77d49b..a37c127f2e03 100644 --- a/pkg/user/initializer.go +++ b/pkg/user/initializer.go @@ -1,8 +1,6 @@ package user -import ( - "github.com/openshift/origin/pkg/user/api" -) +import "github.com/openshift/origin/pkg/user/api" type DefaultUserInitStrategy struct { } @@ -13,7 +11,6 @@ func NewDefaultUserInitStrategy() Initializer { // InitializeUser implements Initializer func (*DefaultUserInitStrategy) InitializeUser(identity *api.Identity, user *api.User) error { - user.FullName = identity.UserName if identity.Extra != nil { if name, ok := identity.Extra["name"]; ok && len(name) > 0 { user.FullName = name diff --git a/pkg/user/registry/etcd/etcd.go b/pkg/user/registry/etcd/etcd.go deleted file mode 100644 index 87edcf263fcc..000000000000 --- a/pkg/user/registry/etcd/etcd.go +++ /dev/null @@ -1,115 +0,0 @@ -package etcd - -import ( - "errors" - "fmt" - - etcderrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" - - "github.com/openshift/origin/pkg/user" - "github.com/openshift/origin/pkg/user/api" -) - -// Etcd implements UserIdentityMapping backed by etcd. -type Etcd struct { - tools.EtcdHelper - initializer user.Initializer -} - -// New returns a new Etcd. -func New(helper tools.EtcdHelper, initializer user.Initializer) *Etcd { - return &Etcd{ - EtcdHelper: helper, - initializer: initializer, - } -} - -var errExists = errors.New("the mapping already exists") - -func makeUserKey(id string) string { - return "/userIdentityMappings/" + id -} - -func (r *Etcd) GetUser(name string) (user *api.User, err error) { - mapping := &api.UserIdentityMapping{} - err = r.ExtractObj(makeUserKey(name), mapping, false) - err = etcderrs.InterpretGetError(err, "User", name) - user = &mapping.User - return -} - -func (r *Etcd) GetUserIdentityMapping(name string) (mapping *api.UserIdentityMapping, err error) { - mapping = &api.UserIdentityMapping{} - err = r.ExtractObj(makeUserKey(name), mapping, false) - err = etcderrs.InterpretGetError(err, "UserIdentityMapping", name) - return -} - -// CreateOrUpdateUserIdentityMapping implements useridentitymapping.Registry -func (r *Etcd) CreateOrUpdateUserIdentityMapping(mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, bool, error) { - // Create Identity.Name by combining Provider and UserName - name := fmt.Sprintf("%s:%s", mapping.Identity.Provider, mapping.Identity.UserName) - key := makeUserKey(name) - - // track the object we set into etcd to return - var found *api.UserIdentityMapping - var created bool - - err := r.AtomicUpdate(key, &api.UserIdentityMapping{}, true, func(in runtime.Object) (runtime.Object, uint64, error) { - existing := *in.(*api.UserIdentityMapping) - - // did not previously exist - if existing.Name == "" { - now := util.Now() - - // TODO: move these initializations the rest layer once we stop using the registry directly - existing.Name = name - existing.UID = util.NewUUID() - existing.CreationTimestamp = now - existing.Identity = mapping.Identity - - identityuid := util.NewUUID() - existing.Identity.Name = name - existing.Identity.UID = identityuid - existing.Identity.CreationTimestamp = now - - useruid := util.NewUUID() - existing.User.Name = name - existing.User.UID = useruid - existing.User.CreationTimestamp = now - - if err := r.initializer.InitializeUser(&existing.Identity, &existing.User); err != nil { - return in, 0, err - } - - // set these again to prevent bad initialization from messing up data - existing.Identity.Name = name - existing.Identity.UID = identityuid - existing.Identity.CreationTimestamp = now - existing.User.Name = name - existing.User.UID = useruid - existing.User.CreationTimestamp = now - - found = &existing - created = true - return &existing, 0, nil - } - - if existing.User.Name != name { - return in, 0, fmt.Errorf("the provided user name does not match the existing mapping %s", existing.User.Name) - } - found = &existing - - // TODO: should update identity based on new info as well. - return in, 0, errExists - }) - - if err != nil && err != errExists { - err = etcderrs.InterpretCreateError(err, "UserIdentityMapping", name) - return nil, false, err - } - return found, created, nil -} diff --git a/pkg/user/registry/etcd/etcd_test.go b/pkg/user/registry/etcd/etcd_test.go deleted file mode 100644 index 757316914280..000000000000 --- a/pkg/user/registry/etcd/etcd_test.go +++ /dev/null @@ -1,160 +0,0 @@ -package etcd - -import ( - "fmt" - "strings" - "testing" - - kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" - "github.com/coreos/go-etcd/etcd" - - "github.com/openshift/origin/pkg/api/latest" - "github.com/openshift/origin/pkg/user" - userapi "github.com/openshift/origin/pkg/user/api" -) - -func NewTestEtcd(client tools.EtcdClient) *Etcd { - return New(tools.NewEtcdHelper(client, latest.Codec), user.NewDefaultUserInitStrategy()) -} - -// This copy and paste is not pure ignorance. This is that we can be sure that the key is getting made as we -// expect it to. If someone changes the location of these resources by say moving all the resources to -// "/origin/resources" (which is a really good idea), then they've made a breaking change and something should -// fail to let them know they've change some significant change and that other dependent pieces may break. -func makeTestUserIdentityMapping(providerID, userName string) string { - return fmt.Sprintf("/userIdentityMappings/%s:%s", providerID, userName) -} - -func TestEtcdGetUser(t *testing.T) { - fakeClient := tools.NewFakeEtcdClient(t) - expectedResultingMapping := &userapi.UserIdentityMapping{ - Identity: userapi.Identity{ - ObjectMeta: kapi.ObjectMeta{Name: "victor:tango"}, - Provider: "victor", - UserName: "tango", - }, - User: userapi.User{ - ObjectMeta: kapi.ObjectMeta{Name: "uniform"}, - }, - } - key := makeTestUserIdentityMapping(expectedResultingMapping.Identity.Provider, expectedResultingMapping.Identity.UserName) - fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, expectedResultingMapping), 0) - registry := NewTestEtcd(fakeClient) - - user, err := registry.GetUser("victor:tango") - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if user.Name != expectedResultingMapping.User.Name { - t.Errorf("Expected %#v, but we got %#v", expectedResultingMapping.User.Name, user.Name) - } -} - -func TestEtcdCreateUserIdentityMapping(t *testing.T) { - fakeClient := tools.NewFakeEtcdClient(t) - fakeClient.TestIndex = true - - testMapping := &userapi.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{Name: "quebec"}, - Identity: userapi.Identity{ - Provider: "sierra", - }, - User: userapi.User{}, - } - expectedResultingMapping := &userapi.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{Name: "quebec"}, - Identity: userapi.Identity{ - ObjectMeta: kapi.ObjectMeta{Name: "sierra:"}, - Provider: "sierra", - }, - User: userapi.User{ - ObjectMeta: kapi.ObjectMeta{Name: "romeo"}, - }, - } - key := makeTestUserIdentityMapping(testMapping.Identity.Provider, testMapping.Identity.UserName) - - fakeClient.Data[key] = tools.EtcdResponseWithError{ - R: &etcd.Response{ - Node: nil, - }, - E: tools.EtcdErrorNotFound, - } - registry := NewTestEtcd(fakeClient) - persistedUserIdentityMapping, created, err := registry.CreateOrUpdateUserIdentityMapping(testMapping) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !created { - t.Errorf("Expected to be created, but we were updated instead") - } - if persistedUserIdentityMapping == nil { - t.Errorf("Expected %#v, but we got %#v", expectedResultingMapping, persistedUserIdentityMapping) - } - if compareUserIdentityMappingFieldsThatAreFixed(expectedResultingMapping, persistedUserIdentityMapping) { - t.Errorf("Expected %#v, but we got %#v", expectedResultingMapping, persistedUserIdentityMapping) - } -} - -func TestEtcdUpdateUserIdentityMappingWithConflictingUser(t *testing.T) { - fakeClient := tools.NewFakeEtcdClient(t) - startingMapping := &userapi.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{Name: "whiskey"}, - Identity: userapi.Identity{ - ObjectMeta: kapi.ObjectMeta{Name: "yankee:xray"}, - Provider: "yankee", - UserName: "xray", - }, - User: userapi.User{ - ObjectMeta: kapi.ObjectMeta{Name: "yankee:xray"}, - }, - } - // this key is intentionally wrong so that we can have an internally consistent UserIdentityMapping - // that was placed in a bad key location - key := makeTestUserIdentityMapping("zulu", "alfa") - fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, startingMapping), 0) - registry := NewTestEtcd(fakeClient) - - testMapping := &userapi.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{Name: "bravo"}, - Identity: userapi.Identity{ - Provider: "zulu", - UserName: "alfa", - }, - User: userapi.User{}, - } - - persistedUserIdentityMapping, created, err := registry.CreateOrUpdateUserIdentityMapping(testMapping) - if err == nil { - t.Errorf("Expected an error, but we didn't get one") - } else { - const expectedError = "the provided user name does not match the existing mapping" - if !strings.Contains(err.Error(), expectedError) { - t.Errorf("Expected error %v, but we got %v", expectedError, expectedError) - } - if created { - t.Errorf("Expected be updated, but we were created instead") - } - if persistedUserIdentityMapping != nil { - t.Errorf("Expected nil, but we got %#v", persistedUserIdentityMapping) - } - } -} - -func compareUserIdentityMappingFieldsThatAreFixed(expected, actual *userapi.UserIdentityMapping) bool { - if ((actual == nil) && (expected != nil)) || ((actual != nil) && (expected == nil)) { - return false - } - if actual.Name != expected.Name { - return false - } - if actual.Identity.UserName != expected.Identity.UserName || actual.Identity.Provider != expected.Identity.Provider { - return false - } - if actual.User.Name != expected.User.Name || actual.User.Name != actual.User.Name { - return false - } - - return true -} diff --git a/pkg/user/registry/identity/etcd/etcd.go b/pkg/user/registry/identity/etcd/etcd.go new file mode 100644 index 000000000000..edac8122aadb --- /dev/null +++ b/pkg/user/registry/identity/etcd/etcd.go @@ -0,0 +1,49 @@ +package etcd + +import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" + etcdgeneric "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/identity" +) + +// rest implements a RESTStorage for identites against etcd +type REST struct { + etcdgeneric.Etcd +} + +const EtcdPrefix = "/registry/identities" + +// NewREST returns a RESTStorage object that will work against identites +func NewREST(h tools.EtcdHelper) *REST { + store := &etcdgeneric.Etcd{ + NewFunc: func() runtime.Object { return &api.Identity{} }, + NewListFunc: func() runtime.Object { return &api.IdentityList{} }, + KeyRootFunc: func(ctx kapi.Context) string { + return EtcdPrefix + }, + KeyFunc: func(ctx kapi.Context, name string) (string, error) { + return etcdgeneric.NoNamespaceKeyFunc(ctx, EtcdPrefix, name) + }, + ObjectNameFunc: func(obj runtime.Object) (string, error) { + return obj.(*api.Identity).Name, nil + }, + PredicateFunc: func(label labels.Selector, field fields.Selector) generic.Matcher { + return identity.MatchIdentity(label, field) + }, + EndpointName: "identities", + + Helper: h, + } + + store.CreateStrategy = identity.Strategy + store.UpdateStrategy = identity.Strategy + + return &REST{*store} +} diff --git a/pkg/user/registry/identity/registry.go b/pkg/user/registry/identity/registry.go index 2dd526cc989c..5a9b4fe374ee 100644 --- a/pkg/user/registry/identity/registry.go +++ b/pkg/user/registry/identity/registry.go @@ -1,5 +1,81 @@ package identity -// Registry is an interface for things that know how to store Identity objects. +import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/openshift/origin/pkg/user/api" +) + +// Registry is an interface implemented by things that know how to store Identity objects. type Registry interface { + // ListIdentities obtains a list of Identities having labels which match selector. + ListIdentities(ctx kapi.Context, selector labels.Selector) (*api.IdentityList, error) + // GetIdentity returns a specific Identity + GetIdentity(ctx kapi.Context, name string) (*api.Identity, error) + // CreateIdentity creates a Identity + CreateIdentity(ctx kapi.Context, Identity *api.Identity) (*api.Identity, error) + // UpdateIdentity updates an existing Identity + UpdateIdentity(ctx kapi.Context, Identity *api.Identity) (*api.Identity, error) +} + +func identityName(provider, identity string) string { + // TODO: normalize? + return provider + ":" + identity +} + +// Storage is an interface for a standard REST Storage backend +// TODO: move me somewhere common +type Storage interface { + apiserver.RESTLister + apiserver.RESTGetter + + Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, error) + Update(ctx kapi.Context, obj runtime.Object) (runtime.Object, bool, error) +} + +// storage puts strong typing around storage calls +type storage struct { + Storage +} + +// NewRegistry returns a new Registry interface for the given Storage. Any mismatched +// types will panic. +func NewRegistry(s Storage) Registry { + return &storage{s} +} + +func (s *storage) ListIdentities(ctx kapi.Context, label labels.Selector) (*api.IdentityList, error) { + obj, err := s.List(ctx, label, fields.Everything()) + if err != nil { + return nil, err + } + return obj.(*api.IdentityList), nil +} + +func (s *storage) GetIdentity(ctx kapi.Context, name string) (*api.Identity, error) { + obj, err := s.Get(ctx, name) + if err != nil { + return nil, err + } + return obj.(*api.Identity), nil +} + +func (s *storage) CreateIdentity(ctx kapi.Context, Identity *api.Identity) (*api.Identity, error) { + obj, err := s.Create(ctx, Identity) + if err != nil { + return nil, err + } + return obj.(*api.Identity), nil +} + +func (s *storage) UpdateIdentity(ctx kapi.Context, Identity *api.Identity) (*api.Identity, error) { + obj, _, err := s.Update(ctx, Identity) + if err != nil { + return nil, err + } + return obj.(*api.Identity), nil } diff --git a/pkg/user/registry/identity/rest.go b/pkg/user/registry/identity/rest.go new file mode 100644 index 000000000000..923b0a82366f --- /dev/null +++ b/pkg/user/registry/identity/rest.go @@ -0,0 +1,77 @@ +package identity + +import ( + "fmt" + + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/api/validation" +) + +// identityStrategy implements behavior for Identities +type identityStrategy struct { + runtime.ObjectTyper +} + +// Strategy is the default logic that applies when creating and updating Identity +// objects via the REST API. +var Strategy = identityStrategy{kapi.Scheme} + +// NamespaceScoped is false for users +func (identityStrategy) NamespaceScoped() bool { + return false +} + +func (identityStrategy) GenerateName(base string) string { + return base +} + +func (identityStrategy) ResetBeforeCreate(obj runtime.Object) { + identity := obj.(*api.Identity) + identity.Name = identityName(identity.ProviderName, identity.ProviderUserName) +} + +// Validate validates a new user +func (identityStrategy) Validate(obj runtime.Object) kerrs.ValidationErrorList { + identity := obj.(*api.Identity) + return validation.ValidateIdentity(identity) +} + +// AllowCreateOnUpdate is false for identity +func (identityStrategy) AllowCreateOnUpdate() bool { + return false +} + +// ValidateUpdate is the default update validation for an identity +func (identityStrategy) ValidateUpdate(obj, old runtime.Object) kerrs.ValidationErrorList { + return validation.ValidateIdentityUpdate(obj.(*api.Identity), old.(*api.Identity)) +} + +// MatchIdentity returns a generic matcher for a given label and field selector. +func MatchIdentity(label labels.Selector, field fields.Selector) generic.Matcher { + return generic.MatcherFunc(func(obj runtime.Object) (bool, error) { + identityObj, ok := obj.(*api.Identity) + if !ok { + return false, fmt.Errorf("not an identity") + } + fields := IdentityToSelectableFields(identityObj) + return label.Matches(labels.Set(identityObj.Labels)) && field.Matches(fields), nil + }) +} + +// IdentityToSelectableFields returns a label set that represents the object +func IdentityToSelectableFields(identity *api.Identity) labels.Set { + return labels.Set{ + "name": identity.Name, + "providerName": identity.ProviderName, + "providerUserName": identity.ProviderName, + "user.name": identity.User.Name, + "user.uid": string(identity.User.UID), + } +} diff --git a/pkg/user/registry/test/identity.go b/pkg/user/registry/test/identity.go new file mode 100644 index 000000000000..68764d12194d --- /dev/null +++ b/pkg/user/registry/test/identity.go @@ -0,0 +1,73 @@ +package test + +import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + + "github.com/openshift/origin/pkg/user/api" +) + +type Action struct { + Name string + Object interface{} +} + +type IdentityRegistry struct { + GetErr map[string]error + Get map[string]*api.Identity + + CreateErr error + Create *api.Identity + + UpdateErr error + Update *api.Identity + + ListErr error + List *api.IdentityList + + Actions *[]Action +} + +func NewIdentityRegistry() *IdentityRegistry { + return &IdentityRegistry{ + GetErr: map[string]error{}, + Get: map[string]*api.Identity{}, + Actions: &[]Action{}, + } +} + +func (r *IdentityRegistry) GetIdentity(ctx kapi.Context, name string) (*api.Identity, error) { + *r.Actions = append(*r.Actions, Action{"GetIdentity", name}) + if identity, ok := r.Get[name]; ok { + return identity, nil + } + if err, ok := r.GetErr[name]; ok { + return nil, err + } + return nil, kerrs.NewNotFound("Identity", name) +} + +func (r *IdentityRegistry) CreateIdentity(ctx kapi.Context, u *api.Identity) (*api.Identity, error) { + *r.Actions = append(*r.Actions, Action{"CreateIdentity", u}) + if r.Create == nil && r.CreateErr == nil { + return u, nil + } + return r.Create, r.CreateErr +} + +func (r *IdentityRegistry) UpdateIdentity(ctx kapi.Context, u *api.Identity) (*api.Identity, error) { + *r.Actions = append(*r.Actions, Action{"UpdateIdentity", u}) + if r.Update == nil && r.UpdateErr == nil { + return u, nil + } + return r.Update, r.UpdateErr +} + +func (r *IdentityRegistry) ListIdentities(ctx kapi.Context, labels labels.Selector) (*api.IdentityList, error) { + *r.Actions = append(*r.Actions, Action{"ListIdentities", labels}) + if r.List == nil && r.ListErr == nil { + return &api.IdentityList{}, nil + } + return r.List, r.ListErr +} diff --git a/pkg/user/registry/test/user.go b/pkg/user/registry/test/user.go index e266f9a3bbc8..a7c20aa86ca7 100644 --- a/pkg/user/registry/test/user.go +++ b/pkg/user/registry/test/user.go @@ -1,15 +1,70 @@ package test import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/openshift/origin/pkg/user/api" ) type UserRegistry struct { - Err error - User *api.User - DeletedUserID string + GetErr map[string]error + Get map[string]*api.User + + CreateErr error + Create *api.User + + UpdateErr map[string]error + Update *api.User + + ListErr error + List *api.UserList + + Actions *[]Action +} + +func NewUserRegistry() *UserRegistry { + return &UserRegistry{ + GetErr: map[string]error{}, + Get: map[string]*api.User{}, + UpdateErr: map[string]error{}, + Actions: &[]Action{}, + } +} + +func (r *UserRegistry) GetUser(ctx kapi.Context, name string) (*api.User, error) { + *r.Actions = append(*r.Actions, Action{"GetUser", name}) + if user, ok := r.Get[name]; ok { + return user, nil + } + if err, ok := r.GetErr[name]; ok { + return nil, err + } + return nil, kerrs.NewNotFound("User", name) +} + +func (r *UserRegistry) CreateUser(ctx kapi.Context, u *api.User) (*api.User, error) { + *r.Actions = append(*r.Actions, Action{"CreateUser", u}) + if r.Create == nil && r.CreateErr == nil { + return u, nil + } + return r.Create, r.CreateErr +} + +func (r *UserRegistry) UpdateUser(ctx kapi.Context, u *api.User) (*api.User, error) { + *r.Actions = append(*r.Actions, Action{"UpdateUser", u}) + err, _ := r.UpdateErr[u.Name] + if r.Update == nil && err == nil { + return u, nil + } + return r.Update, err } -func (r *UserRegistry) GetUser(id string) (*api.User, error) { - return r.User, r.Err +func (r *UserRegistry) ListUsers(ctx kapi.Context, labels labels.Selector) (*api.UserList, error) { + *r.Actions = append(*r.Actions, Action{"ListUsers", labels}) + if r.List == nil && r.ListErr == nil { + return &api.UserList{}, nil + } + return r.List, r.ListErr } diff --git a/pkg/user/registry/user/etcd/etcd.go b/pkg/user/registry/user/etcd/etcd.go new file mode 100644 index 000000000000..a55f979515a8 --- /dev/null +++ b/pkg/user/registry/user/etcd/etcd.go @@ -0,0 +1,70 @@ +package etcd + +import ( + "errors" + + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" + etcdgeneric "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/api/validation" + "github.com/openshift/origin/pkg/user/registry/user" +) + +// rest implements a RESTStorage for users against etcd +type REST struct { + etcdgeneric.Etcd +} + +const EtcdPrefix = "/registry/users" + +// NewREST returns a RESTStorage object that will work against users +func NewREST(h tools.EtcdHelper) *REST { + store := &etcdgeneric.Etcd{ + NewFunc: func() runtime.Object { return &api.User{} }, + NewListFunc: func() runtime.Object { return &api.UserList{} }, + KeyRootFunc: func(ctx kapi.Context) string { + return EtcdPrefix + }, + KeyFunc: func(ctx kapi.Context, name string) (string, error) { + return etcdgeneric.NoNamespaceKeyFunc(ctx, EtcdPrefix, name) + }, + ObjectNameFunc: func(obj runtime.Object) (string, error) { + return obj.(*api.User).Name, nil + }, + PredicateFunc: func(label labels.Selector, field fields.Selector) generic.Matcher { + return user.MatchUser(label, field) + }, + EndpointName: "users", + + Helper: h, + } + + store.CreateStrategy = user.Strategy + store.UpdateStrategy = user.Strategy + + return &REST{*store} +} + +// Get retrieves the item from etcd. +func (r *REST) Get(ctx kapi.Context, name string) (runtime.Object, error) { + // "~" means the currently authenticated user + if name == "~" { + user, ok := kapi.UserFrom(ctx) + if !ok || user.GetName() == "" { + return nil, kerrs.NewForbidden("user", "~", errors.New("Requests to ~ must be authenticated")) + } + name = user.GetName() + } + if ok, details := validation.ValidateUserName(name, false); !ok { + return nil, kerrs.NewFieldInvalid("metadata.name", name, details) + } + + return r.Etcd.Get(ctx, name) +} diff --git a/pkg/user/registry/user/registry.go b/pkg/user/registry/user/registry.go index 8faf9136081d..1a7c9d795196 100644 --- a/pkg/user/registry/user/registry.go +++ b/pkg/user/registry/user/registry.go @@ -1,10 +1,76 @@ package user import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/openshift/origin/pkg/user/api" ) -// Registry is an interface for things that know how to store User objects. +// Registry is an interface implemented by things that know how to store User objects. type Registry interface { - GetUser(name string) (*api.User, error) + // ListUsers obtains a list of users having labels which match selector. + ListUsers(ctx kapi.Context, selector labels.Selector) (*api.UserList, error) + // GetUser returns a specific user + GetUser(ctx kapi.Context, name string) (*api.User, error) + // CreateUser creates a user + CreateUser(ctx kapi.Context, user *api.User) (*api.User, error) + // UpdateUser updates an existing user + UpdateUser(ctx kapi.Context, user *api.User) (*api.User, error) +} + +// Storage is an interface for a standard REST Storage backend +// TODO: move me somewhere common +type Storage interface { + apiserver.RESTLister + apiserver.RESTGetter + + Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, error) + Update(ctx kapi.Context, obj runtime.Object) (runtime.Object, bool, error) +} + +// storage puts strong typing around storage calls +type storage struct { + Storage +} + +// NewRegistry returns a new Registry interface for the given Storage. Any mismatched +// types will panic. +func NewRegistry(s Storage) Registry { + return &storage{s} +} + +func (s *storage) ListUsers(ctx kapi.Context, label labels.Selector) (*api.UserList, error) { + obj, err := s.List(ctx, label, fields.Everything()) + if err != nil { + return nil, err + } + return obj.(*api.UserList), nil +} + +func (s *storage) GetUser(ctx kapi.Context, name string) (*api.User, error) { + obj, err := s.Get(ctx, name) + if err != nil { + return nil, err + } + return obj.(*api.User), nil +} + +func (s *storage) CreateUser(ctx kapi.Context, user *api.User) (*api.User, error) { + obj, err := s.Create(ctx, user) + if err != nil { + return nil, err + } + return obj.(*api.User), nil +} + +func (s *storage) UpdateUser(ctx kapi.Context, user *api.User) (*api.User, error) { + obj, _, err := s.Update(ctx, user) + if err != nil { + return nil, err + } + return obj.(*api.User), nil } diff --git a/pkg/user/registry/user/rest.go b/pkg/user/registry/user/rest.go index 668ab5e0195e..76bd704b0546 100644 --- a/pkg/user/registry/user/rest.go +++ b/pkg/user/registry/user/rest.go @@ -1,44 +1,71 @@ package user import ( - "errors" + "fmt" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" - "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + "github.com/GoogleCloudPlatform/kubernetes/pkg/fields" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/openshift/origin/pkg/user/api" "github.com/openshift/origin/pkg/user/api/validation" ) -// REST implements the RESTStorage interface in terms of an Registry. -type REST struct { - registry Registry +// userStrategy implements behavior for Users +type userStrategy struct { + runtime.ObjectTyper } -// NewREST returns a new REST. -func NewREST(registry Registry) apiserver.RESTStorage { - return &REST{registry} +// Strategy is the default logic that applies when creating and updating User +// objects via the REST API. +var Strategy = userStrategy{kapi.Scheme} + +// NamespaceScoped is false for users +func (userStrategy) NamespaceScoped() bool { + return false +} + +func (userStrategy) GenerateName(base string) string { + return base +} + +func (userStrategy) ResetBeforeCreate(obj runtime.Object) { +} + +// Validate validates a new user +func (userStrategy) Validate(obj runtime.Object) kerrs.ValidationErrorList { + user := obj.(*api.User) + return validation.ValidateUser(user) +} + +// AllowCreateOnUpdate is false for users +func (userStrategy) AllowCreateOnUpdate() bool { + return false } -// New returns a new UserIdentityMapping for use with Create and Update. -func (s *REST) New() runtime.Object { - return &api.User{} +// ValidateUpdate is the default update validation for an end user. +func (userStrategy) ValidateUpdate(obj, old runtime.Object) kerrs.ValidationErrorList { + return validation.ValidateUserUpdate(obj.(*api.User), old.(*api.User)) } -// Get retrieves an UserIdentityMapping by id. -func (s *REST) Get(ctx kapi.Context, id string) (runtime.Object, error) { - // "~" means the currently authenticated user - if id == "~" { - user, ok := kapi.UserFrom(ctx) - if !ok || user.GetName() == "" { - return nil, kerrs.NewForbidden("user", "~", errors.New("Requests to ~ must be authenticated")) +// MatchUser returns a generic matcher for a given label and field selector. +func MatchUser(label labels.Selector, field fields.Selector) generic.Matcher { + return generic.MatcherFunc(func(obj runtime.Object) (bool, error) { + userObj, ok := obj.(*api.User) + if !ok { + return false, fmt.Errorf("not a user") } - id = user.GetName() - } - if ok, details := validation.ValidateUserName(id, false); !ok { - return nil, kerrs.NewFieldInvalid("metadata.name", id, details) + fields := UserToSelectableFields(userObj) + return label.Matches(labels.Set(userObj.Labels)) && field.Matches(fields), nil + }) +} + +// UserToSelectableFields returns a label set that represents the object +func UserToSelectableFields(user *api.User) labels.Set { + return labels.Set{ + "name": user.Name, } - return s.registry.GetUser(id) } diff --git a/pkg/user/registry/useridentitymapping/registry.go b/pkg/user/registry/useridentitymapping/registry.go index 04624bcfc909..857e19ddb732 100644 --- a/pkg/user/registry/useridentitymapping/registry.go +++ b/pkg/user/registry/useridentitymapping/registry.go @@ -1,13 +1,72 @@ package useridentitymapping import ( + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/openshift/origin/pkg/user/api" ) -// Registry is an interface for things that know how to store Identity objects. +// Registry is an interface implemented by things that know how to store UserIdentityMapping objects. type Registry interface { - GetUserIdentityMapping(name string) (*api.UserIdentityMapping, error) - // CreateOrUpdateUserIdentityMapping creates or updates the mapping between an - // identity and a user. - CreateOrUpdateUserIdentityMapping(mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, bool, error) + // GetUserIdentityMapping returns a UserIdentityMapping for the named identity + GetUserIdentityMapping(ctx kapi.Context, name string) (*api.UserIdentityMapping, error) + // CreateUserIdentityMapping associates a user and an identity + CreateUserIdentityMapping(ctx kapi.Context, mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, error) + // UpdateUserIdentityMapping updates an associated user and identity + UpdateUserIdentityMapping(ctx kapi.Context, mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, error) + // DeleteUserIdentityMapping removes the user association for the named identity + DeleteUserIdentityMapping(ctx kapi.Context, name string) error +} + +// Storage is an interface for a standard REST Storage backend +// TODO: move me somewhere common +type Storage interface { + apiserver.RESTGetter + apiserver.RESTDeleter + + Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, error) + Update(ctx kapi.Context, obj runtime.Object) (runtime.Object, bool, error) +} + +// storage puts strong typing around storage calls +type storage struct { + Storage +} + +// NewRegistry returns a new Registry interface for the given Storage. Any mismatched +// types will panic. +func NewRegistry(s Storage) Registry { + return &storage{s} +} + +func (s *storage) GetUserIdentityMapping(ctx kapi.Context, name string) (*api.UserIdentityMapping, error) { + obj, err := s.Get(ctx, name) + if err != nil { + return nil, err + } + return obj.(*api.UserIdentityMapping), nil +} + +func (s *storage) CreateUserIdentityMapping(ctx kapi.Context, mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, error) { + obj, err := s.Create(ctx, mapping) + if err != nil { + return nil, err + } + return obj.(*api.UserIdentityMapping), nil +} + +func (s *storage) UpdateUserIdentityMapping(ctx kapi.Context, mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, error) { + obj, _, err := s.Update(ctx, mapping) + if err != nil { + return nil, err + } + return obj.(*api.UserIdentityMapping), nil +} + +// +func (s *storage) DeleteUserIdentityMapping(ctx kapi.Context, name string) error { + _, err := s.Delete(ctx, name) + return err } diff --git a/pkg/user/registry/useridentitymapping/rest.go b/pkg/user/registry/useridentitymapping/rest.go index b8ed24a87d02..076af471dcb3 100644 --- a/pkg/user/registry/useridentitymapping/rest.go +++ b/pkg/user/registry/useridentitymapping/rest.go @@ -1,45 +1,387 @@ package useridentitymapping import ( + "errors" "fmt" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" - "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/golang/glog" "github.com/openshift/origin/pkg/user/api" "github.com/openshift/origin/pkg/user/api/validation" + "github.com/openshift/origin/pkg/user/registry/identity" + "github.com/openshift/origin/pkg/user/registry/user" ) -// REST implements the RESTStorage interface in terms of an Registry. +// REST implements the RESTStorage interface in terms of an image registry and +// image repository registry. It only supports the Create method and is used +// to simplify adding a new Image and tag to an ImageRepository. type REST struct { - registry Registry + userRegistry user.Registry + identityRegistry identity.Registry } // NewREST returns a new REST. -func NewREST(registry Registry) apiserver.RESTStorage { - return &REST{registry} +func NewREST(userRegistry user.Registry, identityRegistry identity.Registry) *REST { + return &REST{userRegistry: userRegistry, identityRegistry: identityRegistry} } -// New returns a new UserIdentityMapping for use with Create and Update. -func (s *REST) New() runtime.Object { +// userIdentityMappingStrategy implements behavior for image repository mappings. +type userIdentityMappingStrategy struct { + runtime.ObjectTyper +} + +// Strategy is the default logic that applies when creating UserIdentityMapping +// objects via the REST API. +var Strategy = userIdentityMappingStrategy{kapi.Scheme} + +// New returns a new UserIdentityMapping for use with Create. +func (r *REST) New() runtime.Object { return &api.UserIdentityMapping{} } -// Get retrieves an UserIdentityMapping by id. -func (s *REST) Get(ctx kapi.Context, id string) (runtime.Object, error) { - return s.registry.GetUserIdentityMapping(id) +// NamespaceScoped is true for image repository mappings. +func (s userIdentityMappingStrategy) NamespaceScoped() bool { + return false +} + +func (userIdentityMappingStrategy) GenerateName(base string) string { + return base +} + +func (userIdentityMappingStrategy) AllowCreateOnUpdate() bool { + return true +} + +// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. +func (s userIdentityMappingStrategy) ResetBeforeCreate(obj runtime.Object) { + mapping := obj.(*api.UserIdentityMapping) + + if len(mapping.Name) == 0 { + mapping.Name = mapping.Identity.Name + } + mapping.Namespace = "" + mapping.ResourceVersion = "" + + mapping.Identity.Namespace = "" + mapping.Identity.Kind = "" + mapping.Identity.UID = "" + + mapping.User.Namespace = "" + mapping.User.Kind = "" + mapping.User.UID = "" +} + +// ResetBeforeUpdate clears fields that are not allowed to be set by end users on update +func (s userIdentityMappingStrategy) ResetBeforeUpdate(obj runtime.Object) { + mapping := obj.(*api.UserIdentityMapping) + + if len(mapping.Name) == 0 { + mapping.Name = mapping.Identity.Name + } + mapping.Namespace = "" + + mapping.Identity.Namespace = "" + mapping.Identity.Kind = "" + mapping.Identity.UID = "" + + mapping.User.Namespace = "" + mapping.User.Kind = "" + mapping.User.UID = "" +} + +// Validate validates a new UserIdentityMapping. +func (s userIdentityMappingStrategy) Validate(obj runtime.Object) kerrs.ValidationErrorList { + mapping := obj.(*api.UserIdentityMapping) + return validation.ValidateUserIdentityMapping(mapping) +} + +// Validate validates an updated UserIdentityMapping. +func (s userIdentityMappingStrategy) ValidateUpdate(obj runtime.Object, old runtime.Object) kerrs.ValidationErrorList { + mapping := obj.(*api.UserIdentityMapping) + oldmapping := old.(*api.UserIdentityMapping) + return validation.ValidateUserIdentityMappingUpdate(mapping, oldmapping) +} + +// Get returns the mapping for the named identity +func (s *REST) Get(ctx kapi.Context, name string) (runtime.Object, error) { + _, _, _, _, mapping, err := s.getRelatedObjects(ctx, name) + return mapping, err } -// Update will create or update a UserIdentityMapping +// Create associates a user and identity if they both exist, and the identity is not already mapped to a user +func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, error) { + mapping, ok := obj.(*api.UserIdentityMapping) + if !ok { + return nil, errors.New("Invalid type") + } + Strategy.ResetBeforeCreate(mapping) + createdMapping, _, err := s.createOrUpdate(ctx, obj, true) + return createdMapping, err +} + +// Update associates an identity with a user. +// Both the identity and user must already exist. +// If the identity is associated with another user already, it is disassociated. func (s *REST) Update(ctx kapi.Context, obj runtime.Object) (runtime.Object, bool, error) { mapping, ok := obj.(*api.UserIdentityMapping) if !ok { - return nil, false, fmt.Errorf("not a user identity mapping: %#v", obj) + return nil, false, errors.New("Invalid type") + } + Strategy.ResetBeforeUpdate(mapping) + return s.createOrUpdate(ctx, mapping, false) +} + +func (s *REST) createOrUpdate(ctx kapi.Context, obj runtime.Object, forceCreate bool) (runtime.Object, bool, error) { + mapping := obj.(*api.UserIdentityMapping) + identity, identityErr, oldUser, oldUserErr, oldMapping, oldMappingErr := s.getRelatedObjects(ctx, mapping.Name) + + // Ensure we didn't get any errors other than NotFound errors + if !(oldMappingErr == nil || kerrs.IsNotFound(oldMappingErr)) { + return nil, false, oldMappingErr + } + if !(identityErr == nil || kerrs.IsNotFound(identityErr)) { + return nil, false, identityErr + } + if !(oldUserErr == nil || kerrs.IsNotFound(oldUserErr)) { + return nil, false, oldUserErr + } + + // If we expect to be creating, fail if the mapping already existed + if forceCreate && oldMappingErr == nil { + return nil, false, kerrs.NewAlreadyExists("UserIdentityMapping", oldMapping.Name) + } + + // Allow update to create if missing + creating := forceCreate || kerrs.IsNotFound(oldMappingErr) + if creating { + // Pre-create checks with no access to oldMapping + if err := rest.BeforeCreate(Strategy, ctx, mapping); err != nil { + return nil, false, err + } + + // Ensure resource version is not specified + if len(mapping.ResourceVersion) > 0 { + return nil, false, kerrs.NewNotFound("UserIdentityMapping", mapping.Name) + } + } else { + // Pre-update checks with access to oldMapping + if err := rest.BeforeUpdate(Strategy, ctx, mapping, oldMapping); err != nil { + return nil, false, err + } + + // Ensure resource versions match + if len(mapping.ResourceVersion) > 0 && mapping.ResourceVersion != oldMapping.ResourceVersion { + return nil, false, kerrs.NewConflict("UserIdentityMapping", mapping.Name, fmt.Errorf("the resource was updated to %s", oldMapping.ResourceVersion)) + } + + // If we're "updating" to the user we're already pointing to, we're already done + if mapping.User.Name == oldMapping.User.Name { + return oldMapping, false, nil + } } - if errs := validation.ValidateUserIdentityMapping(mapping); len(errs) > 0 { - return nil, false, errors.NewInvalid("userIdentityMapping", mapping.Name, errs) + + // Validate identity + if kerrs.IsNotFound(identityErr) { + errs := kerrs.ValidationErrorList([]error{ + kerrs.NewFieldInvalid("identity.name", mapping.Identity.Name, "referenced identity does not exist"), + }) + return nil, false, kerrs.NewInvalid("UserIdentityMapping", mapping.Name, errs) + } + + // Get new user + newUser, err := s.userRegistry.GetUser(ctx, mapping.User.Name) + if kerrs.IsNotFound(err) { + errs := kerrs.ValidationErrorList([]error{ + kerrs.NewFieldInvalid("user.name", mapping.User.Name, "referenced user does not exist"), + }) + return nil, false, kerrs.NewInvalid("UserIdentityMapping", mapping.Name, errs) + } + if err != nil { + return nil, false, err + } + + // Update the new user to point at the identity. If this fails, Update is re-entrant + if addIdentityToUser(identity, newUser) { + if _, err := s.userRegistry.UpdateUser(ctx, newUser); err != nil { + return nil, false, err + } } - return s.registry.CreateOrUpdateUserIdentityMapping(mapping) + + // Update the identity to point at the new user. If this fails. Update is re-entrant + if setIdentityUser(identity, newUser) { + if updatedIdentity, err := s.identityRegistry.UpdateIdentity(ctx, identity); err != nil { + return nil, false, err + } else { + identity = updatedIdentity + } + } + + // At this point, the mapping for the identity has been updated to the new user + // Everything past this point is cleanup + + // Update the old user to no longer point at the identity. + // If this fails, log the error, but continue, because Update is no longer re-entrant + if oldUser != nil && removeIdentityFromUser(identity, oldUser) { + if _, err := s.userRegistry.UpdateUser(ctx, oldUser); err != nil { + glog.Errorf("Error removing identity reference %s from user %s: %v", identity.Name, oldUser.Name, err) + } + } + + updatedMapping, err := mappingFor(newUser, identity) + return updatedMapping, creating, err +} + +// Delete deletes the user association for the named identity +func (s *REST) Delete(ctx kapi.Context, name string) (runtime.Object, error) { + identity, _, user, _, _, mappingErr := s.getRelatedObjects(ctx, name) + + if mappingErr != nil { + return nil, mappingErr + } + + // Disassociate the identity with the user first + // If this fails, Delete is re-entrant + if removeIdentityFromUser(identity, user) { + if _, err := s.userRegistry.UpdateUser(ctx, user); err != nil { + return nil, err + } + } + + // Remove the user association from the identity last. + // If this fails, log the error, but continue, because Delete is no longer re-entrant + // At this point, the mapping for the identity no longer exists + if unsetIdentityUser(identity) { + if _, err := s.identityRegistry.UpdateIdentity(ctx, identity); err != nil { + glog.Errorf("Error removing user reference %s from identity %s: %v", user.Name, identity.Name, err) + } + } + + return &kapi.Status{Status: kapi.StatusSuccess}, nil +} + +// getRelatedObjects returns the identity, user, and mapping for the named identity +// a nil mappingErr means all objects were retrieved without errors, and correctly reference each other +func (s *REST) getRelatedObjects(ctx kapi.Context, name string) ( + identity *api.Identity, identityErr error, + user *api.User, userErr error, + mapping *api.UserIdentityMapping, mappingErr error, +) { + // Initialize errors to NotFound + identityErr = kerrs.NewNotFound("Identity", name) + userErr = kerrs.NewNotFound("User", "") + mappingErr = kerrs.NewNotFound("UserIdentityMapping", name) + + // Get identity + identity, identityErr = s.identityRegistry.GetIdentity(ctx, name) + if identityErr != nil { + return + } + if !hasUserMapping(identity) { + return + } + + // Get user + user, userErr = s.userRegistry.GetUser(ctx, identity.User.Name) + if userErr != nil { + return + } + + // Ensure relational integrity + if !identityReferencesUser(identity, user) { + return + } + if !userReferencesIdentity(user, identity) { + return + } + + mapping, mappingErr = mappingFor(user, identity) + + return +} + +// hasUserMapping returns true if the given identity references a user +func hasUserMapping(identity *api.Identity) bool { + return len(identity.User.Name) > 0 +} + +// identityReferencesUser returns true if the identity's user name and uid match the given user +func identityReferencesUser(identity *api.Identity, user *api.User) bool { + return identity.User.Name == user.Name && identity.User.UID == user.UID +} + +// userReferencesIdentity returns true if the user's identity list contains the given identity +func userReferencesIdentity(user *api.User, identity *api.Identity) bool { + return util.NewStringSet(user.Identities...).Has(identity.Name) +} + +// addIdentityToUser adds the given identity to the user's list of identities +// returns true if the user's identity list was modified +func addIdentityToUser(identity *api.Identity, user *api.User) bool { + identities := util.NewStringSet(user.Identities...) + if identities.Has(identity.Name) { + return false + } + identities.Insert(identity.Name) + user.Identities = identities.List() + return true +} + +// removeIdentityFromUser removes the given identity from the user's list of identities +// returns true if the user's identity list was modified +func removeIdentityFromUser(identity *api.Identity, user *api.User) bool { + identities := util.NewStringSet(user.Identities...) + if !identities.Has(identity.Name) { + return false + } + identities.Delete(identity.Name) + user.Identities = identities.List() + return true +} + +// setIdentityUser sets the identity to reference the given user +// returns true if the identity's user reference was modified +func setIdentityUser(identity *api.Identity, user *api.User) bool { + if identityReferencesUser(identity, user) { + return false + } + identity.User = kapi.ObjectReference{ + Name: user.Name, + UID: user.UID, + } + return true +} + +// unsetIdentityUser clears the identity's user reference +// returns true if the identity's user reference was modified +func unsetIdentityUser(identity *api.Identity) bool { + if !hasUserMapping(identity) { + return false + } + identity.User = kapi.ObjectReference{} + return true +} + +// mappingFor returns a UserIdentityMapping for the given user and identity +// The name and resource version of the identity mapping match the identity +func mappingFor(user *api.User, identity *api.Identity) (*api.UserIdentityMapping, error) { + return &api.UserIdentityMapping{ + ObjectMeta: kapi.ObjectMeta{ + Name: identity.Name, + ResourceVersion: identity.ResourceVersion, + UID: identity.UID, + }, + Identity: kapi.ObjectReference{ + Name: identity.Name, + UID: identity.UID, + }, + User: kapi.ObjectReference{ + Name: user.Name, + UID: user.UID, + }, + }, nil } diff --git a/pkg/user/registry/useridentitymapping/rest_test.go b/pkg/user/registry/useridentitymapping/rest_test.go new file mode 100644 index 000000000000..1c59dcff9ad5 --- /dev/null +++ b/pkg/user/registry/useridentitymapping/rest_test.go @@ -0,0 +1,798 @@ +package useridentitymapping + +import ( + "errors" + "fmt" + "reflect" + "testing" + + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/types" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" +) + +var sequence = 0 + +func makeUser() *api.User { + sequence++ + return makeUserFromSequence(sequence) +} + +func makeUserFromSequence(sequence int) *api.User { + userName := fmt.Sprintf("myuser-%d", sequence) + userUID := types.UID(fmt.Sprintf("useruid-%d", sequence)) + return &api.User{ + ObjectMeta: kapi.ObjectMeta{Name: userName, UID: userUID}, + } +} + +func makeIdentity() *api.Identity { + sequence++ + return makeIdentityFromSequence(sequence) +} + +func makeIdentityFromSequence(sequence int) *api.Identity { + providerName := fmt.Sprintf("providername-%d", sequence) + providerUserName := fmt.Sprintf("providerusername-%d", sequence) + identityName := fmt.Sprintf("%s:%s", providerName, providerUserName) + identityUID := types.UID(fmt.Sprintf("identityuid-%d", sequence)) + + return &api.Identity{ + ObjectMeta: kapi.ObjectMeta{Name: identityName, UID: identityUID}, + ProviderName: providerName, + ProviderUserName: providerUserName, + } +} + +func makeAssociated() (*api.User, *api.Identity) { + sequence++ + return associate(makeUserFromSequence(sequence), makeIdentityFromSequence(sequence)) +} + +func makeUnassociated() (*api.User, *api.Identity) { + sequence++ + return makeUserFromSequence(sequence), makeIdentityFromSequence(sequence) +} + +func associate(user *api.User, identity *api.Identity) (*api.User, *api.Identity) { + userCopy := *user + identityCopy := *identity + addIdentityToUser(&identityCopy, &userCopy) + setIdentityUser(&identityCopy, &userCopy) + return &userCopy, &identityCopy +} + +func disassociate(user *api.User, identity *api.Identity) (*api.User, *api.Identity) { + userCopy := *user + identityCopy := *identity + removeIdentityFromUser(&identityCopy, &userCopy) + unsetIdentityUser(&identityCopy) + return &userCopy, &identityCopy +} + +func setupRegistries(identity *api.Identity, users ...*api.User) (*[]test.Action, *test.UserRegistry, *test.IdentityRegistry, *REST) { + actions := &[]test.Action{} + + userRegistry := &test.UserRegistry{ + Get: map[string]*api.User{}, + GetErr: map[string]error{}, + UpdateErr: map[string]error{}, + Actions: actions, + } + for _, user := range users { + userRegistry.Get[user.Name] = user + } + + identityRegistry := &test.IdentityRegistry{ + Get: map[string]*api.Identity{}, + GetErr: map[string]error{}, + Actions: actions, + } + if identity != nil { + identityRegistry.Get[identity.Name] = identity + } + + rest := NewREST(userRegistry, identityRegistry) + + return actions, userRegistry, identityRegistry, rest +} + +func verifyActions(expectedActions []test.Action, actualActions []test.Action, t *testing.T) { + for i, actualAction := range actualActions { + if len(expectedActions) <= i { + t.Errorf("Expected %d actions, got extras: %#v", len(expectedActions), actualActions[i:]) + break + } + expectedAction := expectedActions[i] + if !reflect.DeepEqual(expectedAction, actualAction) { + t.Errorf("Expected\n\t%s %#v\nGot\n\t%s %#v", expectedAction.Name, expectedAction.Object, actualAction.Name, actualAction.Object) + } + } + if len(expectedActions) > len(actualActions) { + t.Errorf("Expected %d actions, missing: %#v", len(expectedActions), expectedActions[len(actualActions):]) + } +} + +func verifyMapping(object runtime.Object, user *api.User, identity *api.Identity, t *testing.T) { + mapping, ok := object.(*api.UserIdentityMapping) + if !ok { + t.Errorf("Expected mapping, got %#v", object) + return + } + if mapping.User.Name != user.Name { + t.Errorf("Expected user.name %s, got %s", user.Name, mapping.User.Name) + } + if mapping.User.UID != user.UID { + t.Errorf("Expected user.uid %s, got %s", user.UID, mapping.User.UID) + } + if mapping.Identity.Name != identity.Name { + t.Errorf("Expected identity.name %s, got %s", identity.Name, mapping.Identity.Name) + } +} + +func TestGet(t *testing.T) { + user, identity := makeAssociated() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + {"GetUser", user.Name}, + } + + actions, _, _, rest := setupRegistries(identity, user) + mapping, err := rest.Get(kapi.NewContext(), identity.Name) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) + verifyMapping(mapping, user, identity, t) +} + +func TestGetMissingIdentity(t *testing.T) { + user, identity := makeAssociated() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + } + + actions, _, _, rest := setupRegistries(nil, user) + _, err := rest.Get(kapi.NewContext(), identity.Name) + + if err == nil { + t.Errorf("Expected error, got none") + } + verifyActions(expectedActions, *actions, t) +} + +func TestGetIdentityWithoutUser(t *testing.T) { + identity := makeIdentity() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + } + + actions, _, _, rest := setupRegistries(identity) + _, err := rest.Get(kapi.NewContext(), identity.Name) + + if err == nil { + t.Errorf("Expected error, got none") + } + if !kerrs.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestGetMissingUser(t *testing.T) { + user, identity := makeAssociated() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + {"GetUser", user.Name}, + } + + actions, _, _, rest := setupRegistries(identity) + _, err := rest.Get(kapi.NewContext(), identity.Name) + + if err == nil { + t.Errorf("Expected error, got none") + } + if !kerrs.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestGetUserWithoutIdentity(t *testing.T) { + user, identity := makeAssociated() + user.Identities = []string{} + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + {"GetUser", user.Name}, + } + + actions, _, _, rest := setupRegistries(identity, user) + _, err := rest.Get(kapi.NewContext(), identity.Name) + + if err == nil { + t.Errorf("Expected error, got none") + } + if !kerrs.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestCreate(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + unassociatedUser, unassociatedIdentity := disassociate(associatedUser, associatedIdentity) + expectedActions := []test.Action{ + {"GetIdentity", unassociatedIdentity.Name}, + {"GetUser", unassociatedUser.Name}, + {"UpdateUser", associatedUser}, + {"UpdateIdentity", associatedIdentity}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity.Name}, + User: kapi.ObjectReference{Name: unassociatedUser.Name}, + } + + actions, _, _, rest := setupRegistries(unassociatedIdentity, unassociatedUser) + createdMapping, err := rest.Create(kapi.NewContext(), mapping) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) + verifyMapping(createdMapping, associatedUser, associatedIdentity, t) +} + +func TestCreateExists(t *testing.T) { + user, identity := makeAssociated() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + {"GetUser", user.Name}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: identity.Name}, + User: kapi.ObjectReference{Name: user.Name}, + } + + actions, _, _, rest := setupRegistries(identity, user) + _, err := rest.Create(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error, got none") + } + if !kerrs.IsAlreadyExists(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestCreateMissingIdentity(t *testing.T) { + user, identity := makeUnassociated() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: identity.Name}, + User: kapi.ObjectReference{Name: user.Name}, + } + + actions, _, _, rest := setupRegistries(nil, user) + _, err := rest.Create(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error, got none") + } + if !kerrs.IsInvalid(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestCreateMissingUser(t *testing.T) { + user, identity := makeUnassociated() + expectedActions := []test.Action{ + {"GetIdentity", identity.Name}, + {"GetUser", user.Name}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: identity.Name}, + User: kapi.ObjectReference{Name: user.Name}, + } + + actions, _, _, rest := setupRegistries(identity) + _, err := rest.Create(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error, got none") + } + if !kerrs.IsInvalid(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestCreateUserUpdateError(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + unassociatedUser, unassociatedIdentity := disassociate(associatedUser, associatedIdentity) + expectedActions := []test.Action{ + {"GetIdentity", unassociatedIdentity.Name}, + {"GetUser", unassociatedUser.Name}, + {"UpdateUser", associatedUser}, + } + expectedErr := errors.New("Update error") + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity.Name}, + User: kapi.ObjectReference{Name: unassociatedUser.Name}, + } + + actions, userRegistry, _, rest := setupRegistries(unassociatedIdentity, unassociatedUser) + userRegistry.UpdateErr[associatedUser.Name] = expectedErr + _, err := rest.Create(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error, got none") + } + if err != expectedErr { + t.Errorf("Unexpected error: %#v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestCreateIdentityUpdateError(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + unassociatedUser, unassociatedIdentity := disassociate(associatedUser, associatedIdentity) + expectedActions := []test.Action{ + {"GetIdentity", unassociatedIdentity.Name}, + {"GetUser", unassociatedUser.Name}, + {"UpdateUser", associatedUser}, + {"UpdateIdentity", associatedIdentity}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity.Name}, + User: kapi.ObjectReference{Name: unassociatedUser.Name}, + } + + actions, _, identityRegistry, rest := setupRegistries(unassociatedIdentity, unassociatedUser) + identityRegistry.UpdateErr = errors.New("Update error") + _, err := rest.Create(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error, got none") + } + if err != identityRegistry.UpdateErr { + t.Errorf("Unexpected error: %#v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestUpdate(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + unassociatedUser2 := makeUser() + // Finishing conditions + unassociatedUser1, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + associatedUser2, associatedIdentity1User2 := associate(unassociatedUser2, unassociatedIdentity1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + // New user lookup + {"GetUser", unassociatedUser2.Name}, + // New user update + {"UpdateUser", associatedUser2}, + // Identity update + {"UpdateIdentity", associatedIdentity1User2}, + // Old user cleanup + {"UpdateUser", unassociatedUser1}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, _, _, rest := setupRegistries(associatedIdentity1User1, associatedUser1, unassociatedUser2) + createdMapping, created, err := rest.Update(kapi.NewContext(), mapping) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if created { + t.Errorf("Unexpected created") + } + verifyActions(expectedActions, *actions, t) + verifyMapping(createdMapping, associatedUser2, associatedIdentity1User2, t) +} + +func TestUpdateMissingIdentity(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + unassociatedUser2 := makeUser() + // Finishing conditions + _, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, _, _, rest := setupRegistries(nil, associatedUser1, unassociatedUser2) + _, _, err := rest.Update(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error: %v", err) + } + if !kerrs.IsInvalid(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestUpdateMissingUser(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + unassociatedUser2 := makeUser() + // Finishing conditions + _, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + // New user lookup + {"GetUser", unassociatedUser2.Name}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, _, _, rest := setupRegistries(associatedIdentity1User1, associatedUser1) + _, _, err := rest.Update(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error: %v", err) + } + if !kerrs.IsInvalid(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestUpdateOldUserMatches(t *testing.T) { + user, identity := makeAssociated() + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", identity.Name}, + {"GetUser", user.Name}, + } + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: identity.Name}, + User: kapi.ObjectReference{Name: user.Name}, + } + + actions, _, _, rest := setupRegistries(identity, user) + createdMapping, created, err := rest.Update(kapi.NewContext(), mapping) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if created { + t.Errorf("Unexpected created") + } + verifyActions(expectedActions, *actions, t) + verifyMapping(createdMapping, user, identity, t) +} + +func TestUpdateWithMatchingResourceVersion(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + associatedIdentity1User1.ResourceVersion = "ver1" + unassociatedUser2 := makeUser() + // Finishing conditions + unassociatedUser1, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + associatedUser2, associatedIdentity1User2 := associate(unassociatedUser2, unassociatedIdentity1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + // New user lookup + {"GetUser", unassociatedUser2.Name}, + // New user update + {"UpdateUser", associatedUser2}, + // Identity update + {"UpdateIdentity", associatedIdentity1User2}, + // Old user cleanup + {"UpdateUser", unassociatedUser1}, + } + + mapping := &api.UserIdentityMapping{ + ObjectMeta: kapi.ObjectMeta{ResourceVersion: "ver1"}, + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, _, _, rest := setupRegistries(associatedIdentity1User1, associatedUser1, unassociatedUser2) + createdMapping, created, err := rest.Update(kapi.NewContext(), mapping) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if created { + t.Errorf("Unexpected created") + } + verifyActions(expectedActions, *actions, t) + verifyMapping(createdMapping, associatedUser2, associatedIdentity1User2, t) +} + +func TestUpdateWithMismatchedResourceVersion(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + associatedIdentity1User1.ResourceVersion = "ver1" + unassociatedUser2 := makeUser() + // Finishing conditions + _, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + } + + mapping := &api.UserIdentityMapping{ + ObjectMeta: kapi.ObjectMeta{ResourceVersion: "ver2"}, + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, _, _, rest := setupRegistries(associatedIdentity1User1, associatedUser1, unassociatedUser2) + _, _, err := rest.Update(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error") + } + if !kerrs.IsConflict(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestUpdateOldUserUpdateError(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + unassociatedUser2 := makeUser() + // Finishing conditions + unassociatedUser1, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + associatedUser2, associatedIdentity1User2 := associate(unassociatedUser2, unassociatedIdentity1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + // New user lookup + {"GetUser", unassociatedUser2.Name}, + // New user update + {"UpdateUser", associatedUser2}, + // Identity update + {"UpdateIdentity", associatedIdentity1User2}, + // Old user cleanup + {"UpdateUser", unassociatedUser1}, + } + expectedErr := errors.New("Couldn't update old user") + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, userRegistry, _, rest := setupRegistries(associatedIdentity1User1, associatedUser1, unassociatedUser2) + userRegistry.UpdateErr[unassociatedUser1.Name] = expectedErr + createdMapping, created, err := rest.Update(kapi.NewContext(), mapping) + + // An error cleaning up the old user shouldn't manifest as an update failure, since the mapping was successfully updated + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if created { + t.Errorf("Unexpected created") + } + verifyActions(expectedActions, *actions, t) + verifyMapping(createdMapping, associatedUser2, associatedIdentity1User2, t) +} + +func TestUpdateUserUpdateError(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + unassociatedUser2 := makeUser() + // Finishing conditions + _, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + associatedUser2, _ := associate(unassociatedUser2, unassociatedIdentity1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + // New user lookup + {"GetUser", unassociatedUser2.Name}, + // New user update + {"UpdateUser", associatedUser2}, + } + expectedErr := errors.New("Couldn't update new user") + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, userRegistry, _, rest := setupRegistries(associatedIdentity1User1, associatedUser1, unassociatedUser2) + userRegistry.UpdateErr[associatedUser2.Name] = expectedErr + _, _, err := rest.Update(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error") + } + if err != expectedErr { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestUpdateIdentityUpdateError(t *testing.T) { + // Starting conditions + associatedUser1, associatedIdentity1User1 := makeAssociated() + unassociatedUser2 := makeUser() + // Finishing conditions + _, unassociatedIdentity1 := disassociate(associatedUser1, associatedIdentity1User1) + associatedUser2, associatedIdentity1User2 := associate(unassociatedUser2, unassociatedIdentity1) + + expectedActions := []test.Action{ + // Existing mapping lookup + {"GetIdentity", associatedIdentity1User1.Name}, + {"GetUser", associatedUser1.Name}, + // New user lookup + {"GetUser", unassociatedUser2.Name}, + // New user update + {"UpdateUser", associatedUser2}, + // Identity update + {"UpdateIdentity", associatedIdentity1User2}, + } + expectedErr := errors.New("Couldn't update identity") + + mapping := &api.UserIdentityMapping{ + Identity: kapi.ObjectReference{Name: unassociatedIdentity1.Name}, + User: kapi.ObjectReference{Name: unassociatedUser2.Name}, + } + + actions, _, identityRegistry, rest := setupRegistries(associatedIdentity1User1, associatedUser1, unassociatedUser2) + identityRegistry.UpdateErr = expectedErr + _, _, err := rest.Update(kapi.NewContext(), mapping) + + if err == nil { + t.Errorf("Expected error") + } + if err != expectedErr { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestDelete(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + unassociatedUser, unassociatedIdentity := disassociate(associatedUser, associatedIdentity) + expectedActions := []test.Action{ + {"GetIdentity", associatedIdentity.Name}, + {"GetUser", associatedUser.Name}, + {"UpdateUser", unassociatedUser}, + {"UpdateIdentity", unassociatedIdentity}, + } + + actions, _, _, rest := setupRegistries(associatedIdentity, associatedUser) + _, err := rest.Delete(kapi.NewContext(), associatedIdentity.Name) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestDeleteMissingIdentity(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + expectedActions := []test.Action{ + {"GetIdentity", associatedIdentity.Name}, + } + + actions, _, _, rest := setupRegistries(nil, associatedUser) + _, err := rest.Delete(kapi.NewContext(), associatedIdentity.Name) + + if err == nil { + t.Errorf("Expected error") + } + if !kerrs.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestDeleteMissingUser(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + expectedActions := []test.Action{ + {"GetIdentity", associatedIdentity.Name}, + {"GetUser", associatedUser.Name}, + } + + actions, _, _, rest := setupRegistries(associatedIdentity) + _, err := rest.Delete(kapi.NewContext(), associatedIdentity.Name) + + if err == nil { + t.Errorf("Expected error") + } + if !kerrs.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestDeleteUserUpdateError(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + unassociatedUser, _ := disassociate(associatedUser, associatedIdentity) + expectedActions := []test.Action{ + {"GetIdentity", associatedIdentity.Name}, + {"GetUser", associatedUser.Name}, + {"UpdateUser", unassociatedUser}, + } + expectedErr := errors.New("Cannot update user") + + actions, userRegistry, _, rest := setupRegistries(associatedIdentity, associatedUser) + userRegistry.UpdateErr[associatedUser.Name] = expectedErr + _, err := rest.Delete(kapi.NewContext(), associatedIdentity.Name) + + if err == nil { + t.Errorf("Expected error") + } + if err != expectedErr { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} + +func TestDeleteIdentityUpdateError(t *testing.T) { + associatedUser, associatedIdentity := makeAssociated() + unassociatedUser, unassociatedIdentity := disassociate(associatedUser, associatedIdentity) + expectedActions := []test.Action{ + {"GetIdentity", associatedIdentity.Name}, + {"GetUser", associatedUser.Name}, + {"UpdateUser", unassociatedUser}, + {"UpdateIdentity", unassociatedIdentity}, + } + expectedErr := errors.New("Cannot update identity") + + actions, _, identityRegistry, rest := setupRegistries(associatedIdentity, associatedUser) + identityRegistry.UpdateErr = expectedErr + _, err := rest.Delete(kapi.NewContext(), associatedIdentity.Name) + + // An error cleaning up the identity reference shouldn't manifest as an update failure, since the mapping no longer exists + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + verifyActions(expectedActions, *actions, t) +} diff --git a/test/integration/auth_proxy_test.go b/test/integration/auth_proxy_test.go index 2217504e6ace..741e2060c397 100644 --- a/test/integration/auth_proxy_test.go +++ b/test/integration/auth_proxy_test.go @@ -24,8 +24,10 @@ import ( oauthetcd "github.com/openshift/origin/pkg/oauth/registry/etcd" "github.com/openshift/origin/pkg/oauth/server/osinserver" "github.com/openshift/origin/pkg/oauth/server/osinserver/registrystorage" - "github.com/openshift/origin/pkg/user" - useretcd "github.com/openshift/origin/pkg/user/registry/etcd" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + identityetcd "github.com/openshift/origin/pkg/user/registry/identity/etcd" + userregistry "github.com/openshift/origin/pkg/user/registry/user" + useretcd "github.com/openshift/origin/pkg/user/registry/user/etcd" testutil "github.com/openshift/origin/test/util" ) @@ -48,11 +50,16 @@ func TestFrontProxyOnAuthorize(t *testing.T) { etcdClient := testutil.NewEtcdClient() etcdHelper, _ := master.NewEtcdHelper(etcdClient, klatest.Version) oauthEtcd := oauthetcd.New(etcdHelper) - userRegistry := useretcd.New(etcdHelper, user.NewDefaultUserInitStrategy()) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper("front-proxy-test" /*for now*/, userRegistry) + + userStorage := useretcd.NewREST(etcdHelper) + userRegistry := userregistry.NewRegistry(userStorage) + identityStorage := identityetcd.NewREST(etcdHelper) + identityRegistry := identityregistry.NewRegistry(identityStorage) + + identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) // this auth request handler is the one that is supposed to recognize information from a front proxy - authRequestHandler := headerrequest.NewAuthenticator(headerrequest.NewDefaultConfig(), identityMapper) + authRequestHandler := headerrequest.NewAuthenticator("front-proxy-test", headerrequest.NewDefaultConfig(), identityMapper) authHandler := &oauthhandlers.EmptyAuth{} storage := registrystorage.New(oauthEtcd, oauthEtcd, oauthEtcd, oauthregistry.NewUserConversion()) diff --git a/test/integration/authorization_test.go b/test/integration/authorization_test.go index b13db1300fac..3980390024c5 100644 --- a/test/integration/authorization_test.go +++ b/test/integration/authorization_test.go @@ -102,7 +102,7 @@ func TestOnlyResolveRolesForBindingsThatMatter(t *testing.T) { RoleName: bootstrappolicy.ViewRoleName, BindingNamespace: bootstrappolicy.DefaultMasterAuthorizationNamespace, Client: clusterAdminClient, - Users: []string{"anypassword:valerie"}, + Users: []string{"valerie"}, } if err := addValerie.Run(); err != nil { t.Errorf("unexpected error: %v", err) @@ -117,7 +117,7 @@ func TestOnlyResolveRolesForBindingsThatMatter(t *testing.T) { RoleName: bootstrappolicy.EditRoleName, BindingNamespace: bootstrappolicy.DefaultMasterAuthorizationNamespace, Client: clusterAdminClient, - Users: []string{"anypassword:edgar"}, + Users: []string{"edgar"}, } if err := addEdgar.Run(); err != nil { t.Errorf("unexpected error: %v", err) @@ -192,7 +192,7 @@ func TestResourceAccessReview(t *testing.T) { RoleName: bootstrappolicy.ViewRoleName, BindingNamespace: "hammer-project", Client: haroldClient, - Users: []string{"anypassword:valerie"}, + Users: []string{"valerie"}, } if err := addValerie.Run(); err != nil { t.Errorf("unexpected error: %v", err) @@ -203,7 +203,7 @@ func TestResourceAccessReview(t *testing.T) { RoleName: bootstrappolicy.EditRoleName, BindingNamespace: "mallet-project", Client: markClient, - Users: []string{"anypassword:edgar"}, + Users: []string{"edgar"}, } if err := addEdgar.Run(); err != nil { t.Errorf("unexpected error: %v", err) @@ -216,7 +216,7 @@ func TestResourceAccessReview(t *testing.T) { clientInterface: haroldClient.ResourceAccessReviews("hammer-project"), review: requestWhoCanViewDeployments, response: authorizationapi.ResourceAccessReviewResponse{ - Users: util.NewStringSet("anypassword:harold", "anypassword:valerie"), + Users: util.NewStringSet("harold", "valerie"), Groups: globalClusterAdminGroups, Namespace: "hammer-project", }, @@ -229,7 +229,7 @@ func TestResourceAccessReview(t *testing.T) { clientInterface: markClient.ResourceAccessReviews("mallet-project"), review: requestWhoCanViewDeployments, response: authorizationapi.ResourceAccessReviewResponse{ - Users: util.NewStringSet("anypassword:mark", "anypassword:edgar"), + Users: util.NewStringSet("mark", "edgar"), Groups: globalClusterAdminGroups, Namespace: "mallet-project", }, @@ -320,7 +320,7 @@ func TestSubjectAccessReview(t *testing.T) { RoleName: bootstrappolicy.ViewRoleName, BindingNamespace: "hammer-project", Client: haroldClient, - Users: []string{"anypassword:valerie"}, + Users: []string{"valerie"}, } if err := addValerie.Run(); err != nil { t.Errorf("unexpected error: %v", err) @@ -331,13 +331,13 @@ func TestSubjectAccessReview(t *testing.T) { RoleName: bootstrappolicy.EditRoleName, BindingNamespace: "mallet-project", Client: markClient, - Users: []string{"anypassword:edgar"}, + Users: []string{"edgar"}, } if err := addEdgar.Run(); err != nil { t.Errorf("unexpected error: %v", err) } - askCanValerieGetProject := &authorizationapi.SubjectAccessReview{User: "anypassword:valerie", Verb: "get", Resource: "projects"} + askCanValerieGetProject := &authorizationapi.SubjectAccessReview{User: "valerie", Verb: "get", Resource: "projects"} subjectAccessReviewTest{ clientInterface: haroldClient.SubjectAccessReviews("hammer-project"), review: askCanValerieGetProject, @@ -357,7 +357,7 @@ func TestSubjectAccessReview(t *testing.T) { }, }.run(t) - askCanEdgarDeletePods := &authorizationapi.SubjectAccessReview{User: "anypassword:edgar", Verb: "delete", Resource: "pods"} + askCanEdgarDeletePods := &authorizationapi.SubjectAccessReview{User: "edgar", Verb: "delete", Resource: "pods"} subjectAccessReviewTest{ clientInterface: markClient.SubjectAccessReviews("mallet-project"), review: askCanEdgarDeletePods, @@ -373,7 +373,7 @@ func TestSubjectAccessReview(t *testing.T) { err: "forbidden", }.run(t) - askCanHaroldUpdateProject := &authorizationapi.SubjectAccessReview{User: "anypassword:harold", Verb: "update", Resource: "projects"} + askCanHaroldUpdateProject := &authorizationapi.SubjectAccessReview{User: "harold", Verb: "update", Resource: "projects"} subjectAccessReviewTest{ clientInterface: haroldClient.SubjectAccessReviews("hammer-project"), review: askCanHaroldUpdateProject, diff --git a/test/integration/cli_get_token_test.go b/test/integration/cli_get_token_test.go index 4d95ab258c50..b4c893ffbc9a 100644 --- a/test/integration/cli_get_token_test.go +++ b/test/integration/cli_get_token_test.go @@ -25,8 +25,10 @@ import ( oauthetcd "github.com/openshift/origin/pkg/oauth/registry/etcd" "github.com/openshift/origin/pkg/oauth/server/osinserver" "github.com/openshift/origin/pkg/oauth/server/osinserver/registrystorage" - "github.com/openshift/origin/pkg/user" - useretcd "github.com/openshift/origin/pkg/user/registry/etcd" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + identityetcd "github.com/openshift/origin/pkg/user/registry/identity/etcd" + userregistry "github.com/openshift/origin/pkg/user/registry/user" + useretcd "github.com/openshift/origin/pkg/user/registry/user/etcd" "github.com/openshift/origin/pkg/cmd/util/clientcmd" "github.com/openshift/origin/pkg/cmd/util/tokencmd" @@ -44,10 +46,15 @@ func TestGetToken(t *testing.T) { etcdClient := testutil.NewEtcdClient() etcdHelper, _ := master.NewEtcdHelper(etcdClient, klatest.Version) oauthEtcd := oauthetcd.New(etcdHelper) - userRegistry := useretcd.New(etcdHelper, user.NewDefaultUserInitStrategy()) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper("front-proxy-test" /*for now*/, userRegistry) - authRequestHandler := basicauthrequest.NewBasicAuthAuthentication(allowanypassword.New(identityMapper), true) + userStorage := useretcd.NewREST(etcdHelper) + userRegistry := userregistry.NewRegistry(userStorage) + identityStorage := identityetcd.NewREST(etcdHelper) + identityRegistry := identityregistry.NewRegistry(identityStorage) + + identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) + + authRequestHandler := basicauthrequest.NewBasicAuthAuthentication(allowanypassword.New("get-token-test", identityMapper), true) authHandler := oauthhandlers.NewUnionAuthenticationHandler( map[string]oauthhandlers.AuthenticationChallenger{"login": passwordchallenger.NewBasicAuthChallenger("openshift")}, nil, nil) @@ -107,7 +114,7 @@ func TestGetToken(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %v", err) } - if token.UserName != "front-proxy-test:user" { + if token.UserName != "user" { t.Errorf("Expected token for \"user\", but got: %#v", token) } } diff --git a/test/integration/login_test.go b/test/integration/login_test.go index 5141290a571b..4c17193fc213 100644 --- a/test/integration/login_test.go +++ b/test/integration/login_test.go @@ -61,7 +61,7 @@ func TestLogin(t *testing.T) { t.Fatalf("Error trying to determine server info: ", err) } - if loginOptions.Username != "anypassword:"+username { + if loginOptions.Username != username { t.Fatalf("Unexpected user after authentication: %#v", loginOptions) } @@ -70,7 +70,7 @@ func TestLogin(t *testing.T) { ProjectName: project, AdminRole: bootstrappolicy.AdminRoleName, MasterPolicyNamespace: bootstrappolicy.DefaultMasterAuthorizationNamespace, - AdminUser: "anypassword:" + username, + AdminUser: username, } if err := newProjectOptions.Run(); err != nil { t.Fatalf("unexpected error, a project is required to continue: ", err) diff --git a/test/integration/oauth_htpasswd_test.go b/test/integration/oauth_htpasswd_test.go index 7634e5de2a9e..b63466fb9368 100644 --- a/test/integration/oauth_htpasswd_test.go +++ b/test/integration/oauth_htpasswd_test.go @@ -72,7 +72,7 @@ func TestHTPasswd(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - if user.Name != "htpasswd:username" { - t.Fatalf("Expected htpasswd:username as the user, got %v", user) + if user.Name != "username" { + t.Fatalf("Expected username as the user, got %v", user) } } diff --git a/test/integration/oauth_request_header_test.go b/test/integration/oauth_request_header_test.go index 8c72937631eb..529c57f6ed23 100644 --- a/test/integration/oauth_request_header_test.go +++ b/test/integration/oauth_request_header_test.go @@ -143,7 +143,7 @@ func TestOAuthRequestHeader(t *testing.T) { t.Fatalf("expected unsuccessful token request, got redirected to %v", redirect.String()) } - // Use the server and CA info, with cert info + // Use the server d CA info, with cert info authProxyConfig := anonConfig authProxyConfig.CertData = clientCert authProxyConfig.KeyData = clientKey @@ -188,7 +188,7 @@ func TestOAuthRequestHeader(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - if user.Name != "requestheader:myuser" { - t.Fatalf("Expected requestheader:myuser as the user, got %v", user) + if user.Name != "myuser" { + t.Fatalf("Expected myuser as the user, got %v", user) } } diff --git a/test/integration/userclient_test.go b/test/integration/userclient_test.go index 93119550bd15..fa41916b22e7 100644 --- a/test/integration/userclient_test.go +++ b/test/integration/userclient_test.go @@ -3,29 +3,22 @@ package integration import ( - "net/http" - "net/http/httptest" - "reflect" + "sync" "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" - "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" - kuser "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" - kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" - "github.com/GoogleCloudPlatform/kubernetes/pkg/master" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" - "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/admit" + "github.com/GoogleCloudPlatform/kubernetes/pkg/types" - "github.com/openshift/origin/pkg/api/latest" - oapauth "github.com/openshift/origin/pkg/auth/authenticator/password/oauthpassword/registry" - "github.com/openshift/origin/pkg/client" - "github.com/openshift/origin/pkg/user" + authapi "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/auth/userregistry/identitymapper" + "github.com/openshift/origin/pkg/cmd/server/etcd" "github.com/openshift/origin/pkg/user/api" - _ "github.com/openshift/origin/pkg/user/api/v1beta1" - "github.com/openshift/origin/pkg/user/registry/etcd" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + identityetcd "github.com/openshift/origin/pkg/user/registry/identity/etcd" userregistry "github.com/openshift/origin/pkg/user/registry/user" + useretcd "github.com/openshift/origin/pkg/user/registry/user/etcd" "github.com/openshift/origin/pkg/user/registry/useridentitymapping" testutil "github.com/openshift/origin/test/util" ) @@ -34,297 +27,231 @@ func init() { testutil.RequireEtcd() } -func TestUserInitialization(t *testing.T) { - testutil.DeleteAllEtcdKeys() - etcdClient := testutil.NewEtcdClient() - interfaces, _ := latest.InterfacesFor(latest.Version) - userRegistry := etcd.New(tools.NewEtcdHelper(etcdClient, interfaces.Codec), user.NewDefaultUserInitStrategy()) - storage := map[string]apiserver.RESTStorage{ - "userIdentityMappings": useridentitymapping.NewREST(userRegistry), - "users": userregistry.NewREST(userRegistry), - } - - osMux := http.NewServeMux() - server := httptest.NewServer(osMux) - defer server.Close() - handlerContainer := master.NewHandlerContainer(osMux) - - version := &apiserver.APIGroupVersion{ - Root: "/osapi", - Version: "v1beta1", - - Storage: storage, - Codec: latest.Codec, - - Mapper: latest.RESTMapper, - - Creater: kapi.Scheme, - Typer: kapi.Scheme, - Linker: interfaces.MetadataAccessor, - - Admit: admit.NewAlwaysAdmit(), - Context: kapi.NewRequestContextMapper(), - } - if err := version.InstallREST(handlerContainer); err != nil { - t.Fatalf("unable to install REST: %v", err) - } - - mapping := api.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{Name: ":test"}, - User: api.User{ - ObjectMeta: kapi.ObjectMeta{Name: ":test"}, - }, - Identity: api.Identity{ - ObjectMeta: kapi.ObjectMeta{Name: ":test"}, - Provider: "", - UserName: "test", - Extra: map[string]string{ - "name": "Mr. Test", - }, - }, - } - - client, err := client.New(&kclient.Config{Host: server.URL}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - actual, created, err := client.UserIdentityMappings().CreateOrUpdate(&mapping) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !created { - t.Errorf("expected created to be true") +func makeIdentityInfo(providerName, providerUserName string, extra map[string]string) authapi.UserIdentityInfo { + info := authapi.NewDefaultUserIdentityInfo("idp", "bob") + if extra != nil { + info.Extra = extra } + return info +} - expectedUser := api.User{ +func makeUser(name string, identities ...string) *api.User { + return &api.User{ ObjectMeta: kapi.ObjectMeta{ - Name: ":test", - // Copy the UID and timestamp from the actual one - UID: actual.User.UID, - CreationTimestamp: actual.User.CreationTimestamp, + Name: name, }, - FullName: "Mr. Test", + Identities: identities, } - // Copy the UID and timestamp from the actual one - mapping.Identity.UID = actual.Identity.UID - mapping.Identity.CreationTimestamp = actual.Identity.CreationTimestamp - expected := &api.UserIdentityMapping{ +} +func makeIdentity(providerName, providerUserName string) *api.Identity { + return &api.Identity{ ObjectMeta: kapi.ObjectMeta{ - Name: ":test", - // Copy the UID and timestamp from the actual one - UID: actual.UID, - CreationTimestamp: actual.CreationTimestamp, + Name: providerName + ":" + providerUserName, }, - Identity: mapping.Identity, - User: expectedUser, + ProviderName: providerName, + ProviderUserName: providerUserName, } - compareIgnoringSelfLinkAndVersion(t, expected, actual) - - // Make sure uid, name, and creation timestamp get initialized - if len(actual.UID) == 0 { - t.Fatalf("Expected UID to be set") - } - if len(actual.Name) == 0 { - t.Fatalf("Expected Name to be set") - } - if actual.CreationTimestamp.IsZero() { - t.Fatalf("Expected CreationTimestamp to be set") - } - if len(actual.User.UID) == 0 { - t.Fatalf("Expected UID to be set") - } - if len(actual.User.Name) == 0 { - t.Fatalf("Expected Name to be set") - } - if actual.User.CreationTimestamp.IsZero() { - t.Fatalf("Expected CreationTimestamp to be set") - } - if len(actual.Identity.UID) == 0 { - t.Fatalf("Expected UID to be set") - } - if len(actual.Identity.Name) == 0 { - t.Fatalf("Expected Name to be set") +} +func makeIdentityWithUserReference(providerName, providerUserName string, userName string, userUID types.UID) *api.Identity { + identity := makeIdentity(providerName, providerUserName) + identity.User.Name = userName + identity.User.UID = userUID + return identity +} +func makeMapping(user, identity string) *api.UserIdentityMapping { + return &api.UserIdentityMapping{ + ObjectMeta: kapi.ObjectMeta{Name: identity}, + User: kapi.ObjectReference{Name: user}, + Identity: kapi.ObjectReference{Name: identity}, } - if actual.Identity.CreationTimestamp.IsZero() { - t.Fatalf("Expected CreationTimestamp to be set") +} + +func TestUserInitialization(t *testing.T) { + + masterConfig, clusterAdminKubeConfig, err := testutil.StartTestMaster() + if err != nil { + t.Fatalf("unexpected error: %v", err) } - user, err := userRegistry.GetUser(expected.User.Name) + clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig) if err != nil { t.Fatalf("unexpected error: %v", err) } - compareIgnoringSelfLinkAndVersion(t, &expected.User, user) - actualUser, err := client.Users().Get(expectedUser.Name) + etcdHelper, err := etcd.NewOpenShiftEtcdHelper(masterConfig.EtcdClientInfo.URL) if err != nil { t.Fatalf("unexpected error: %v", err) } - compareIgnoringSelfLinkAndVersion(t, &expected.User, actualUser) -} -type testTokenSource struct { - Token string - Err error -} + userRegistry := userregistry.NewRegistry(useretcd.NewREST(etcdHelper)) + identityRegistry := identityregistry.NewRegistry(identityetcd.NewREST(etcdHelper)) + useridentityMappingRegistry := useridentitymapping.NewRegistry(useridentitymapping.NewREST(userRegistry, identityRegistry)) -func (s *testTokenSource) AuthenticatePassword(username, password string) (string, bool, error) { - return s.Token, s.Token != "", s.Err -} + lookup := identitymapper.NewLookupIdentityMapper(useridentityMappingRegistry) + provisioner := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) -func TestUserLookup(t *testing.T) { - testutil.DeleteAllEtcdKeys() - etcdClient := testutil.NewEtcdClient() - interfaces, _ := latest.InterfacesFor(latest.Version) - userRegistry := etcd.New(tools.NewEtcdHelper(etcdClient, interfaces.Codec), user.NewDefaultUserInitStrategy()) - userInfo := &kuser.DefaultInfo{ - Name: ":test", - } - contextMapper := kapi.NewRequestContextMapper() + testcases := map[string]struct { + Identity authapi.UserIdentityInfo + Mapper authapi.UserIdentityMapper - storage := map[string]apiserver.RESTStorage{ - "userIdentityMappings": useridentitymapping.NewREST(userRegistry), - "users": userregistry.NewREST(userRegistry), - } + CreateIdentity *api.Identity + CreateUser *api.User + CreateMapping *api.UserIdentityMapping - osMux := http.NewServeMux() - handlerContainer := master.NewHandlerContainer(osMux) + ExpectedErr error + ExpectedUserName string + ExpectedFullName string + }{ + "lookup missing identity": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: lookup, - version := &apiserver.APIGroupVersion{ - Root: "/osapi", - Version: "v1beta1", + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "lookup existing identity": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: lookup, - Storage: storage, - Codec: latest.Codec, + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentity("idp", "bob"), + CreateMapping: makeMapping("mappeduser", "idp:bob"), - Mapper: latest.RESTMapper, + ExpectedUserName: "mappeduser", + }, + "provision missing identity and user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: provisioner, - Creater: kapi.Scheme, - Typer: kapi.Scheme, - Linker: interfaces.MetadataAccessor, + ExpectedUserName: "bob", + }, + "provision missing identity and user with preferred username and display name": { + Identity: makeIdentityInfo("idp", "bob", map[string]string{"name": "Bob, Sr.", "login": "admin"}), + Mapper: provisioner, - Admit: admit.NewAlwaysAdmit(), - Context: contextMapper, - } - if err := version.InstallREST(handlerContainer); err != nil { - t.Fatalf("unable to install REST: %v", err) - } + ExpectedUserName: "admin", + ExpectedFullName: "Bob, Sr.", + }, + "provision missing identity for existing user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: provisioner, - // Wrap with authenticator - authenticatedHandler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - ctx, ok := contextMapper.Get(req) - if !ok { - t.Fatalf("No context on request") - return - } - if err := contextMapper.Update(req, kapi.WithUser(ctx, userInfo)); err != nil { - t.Fatalf("Could not set user on request") - return - } - osMux.ServeHTTP(w, req) - }) + CreateUser: makeUser("bob", "idp:bob"), - // Wrap with contextmapper - contextHandler, err := kapi.NewRequestContextFilter(contextMapper, authenticatedHandler) - if err != nil { - t.Fatalf("Could not create context filter") - } + ExpectedUserName: "bob", + }, + "provision missing identity with conflicting user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: provisioner, - server := httptest.NewServer(contextHandler) + CreateUser: makeUser("bob"), - mapping := api.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{Name: ":test"}, - User: api.User{ - ObjectMeta: kapi.ObjectMeta{Name: ":test"}, - }, - Identity: api.Identity{ - ObjectMeta: kapi.ObjectMeta{Name: ":test"}, - Provider: "", - UserName: "test", - Extra: map[string]string{ - "name": "Mr. Test", - }, + ExpectedUserName: "bob2", }, - } + "provision missing identity with conflicting user and preferred username": { + Identity: makeIdentityInfo("idp", "bob", map[string]string{"login": "admin"}), + Mapper: provisioner, - client, err := client.New(&kclient.Config{Host: server.URL}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + CreateUser: makeUser("admin"), - actual, created, err := client.UserIdentityMappings().CreateOrUpdate(&mapping) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !created { - // TODO: t.Errorf("expected created to be true") - } - expectedUser := api.User{ - ObjectMeta: kapi.ObjectMeta{ - Name: ":test", - // Copy the UID and timestamp from the actual one - UID: actual.User.UID, - CreationTimestamp: actual.User.CreationTimestamp, + ExpectedUserName: "admin2", }, - FullName: "Mr. Test", - } - // Copy the UID and timestamp from the actual one - mapping.Identity.UID = actual.Identity.UID - mapping.Identity.CreationTimestamp = actual.Identity.CreationTimestamp - expected := &api.UserIdentityMapping{ - ObjectMeta: kapi.ObjectMeta{ - Name: ":test", - // Copy the UID and timestamp from the actual one - UID: actual.UID, - CreationTimestamp: actual.CreationTimestamp, + "provision with existing unmapped identity": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: provisioner, + + CreateIdentity: makeIdentity("idp", "bob"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), }, - Identity: mapping.Identity, - User: expectedUser, - } - compareIgnoringSelfLinkAndVersion(t, expected, actual) + "provision with existing mapped identity with invalid user UID": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: provisioner, - // check the user returned by the registry - user, err := userRegistry.GetUser(expected.User.Name) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - compareIgnoringSelfLinkAndVersion(t, &expected.User, user) + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentityWithUserReference("idp", "bob", "mappeduser", "invalidUID"), - // check the user returned by the client - actualUser, err := client.Users().Get(expectedUser.Name) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - compareIgnoringSelfLinkAndVersion(t, &expected.User, actualUser) + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "provision returns existing mapping": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: provisioner, - // check the current user - currentUser, err := client.Users().Get("~") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - compareIgnoringSelfLinkAndVersion(t, &expected.User, currentUser) + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentity("idp", "bob"), + CreateMapping: makeMapping("mappeduser", "idp:bob"), - // test retrieving user info from a token - authorizer := oapauth.New(&testTokenSource{Token: "token"}, server.URL, nil) - info, ok, err := authorizer.AuthenticatePassword("foo", "bar") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !ok { - t.Fatalf("should have been authenticated") - } - if user.Name != info.GetName() || string(user.UID) != info.GetUID() { - t.Errorf("unexpected user info", info) + ExpectedUserName: "mappeduser", + }, } -} -func compareIgnoringSelfLinkAndVersion(t *testing.T, expected runtime.Object, actual runtime.Object) { - if actualAccessor, _ := meta.Accessor(actual); actualAccessor != nil { - actualAccessor.SetSelfLink("") - actualAccessor.SetResourceVersion("") - } + for k, testcase := range testcases { + // Cleanup + if err := etcdHelper.Delete(useretcd.EtcdPrefix, true); err != nil && !tools.IsEtcdNotFound(err) { + t.Fatalf("Could not clean up users: %v", err) + } + if err := etcdHelper.Delete(identityetcd.EtcdPrefix, true); err != nil && !tools.IsEtcdNotFound(err) { + t.Fatalf("Could not clean up identities: %v", err) + } - if !reflect.DeepEqual(expected, actual) { - t.Errorf("expected\n %#v,\n got\n %#v", expected, actual) + // Pre-create items + if testcase.CreateUser != nil { + _, err := clusterAdminClient.Users().Create(testcase.CreateUser) + if err != nil { + t.Errorf("%s: Could not create user: %v", k, err) + continue + } + } + if testcase.CreateIdentity != nil { + _, err := clusterAdminClient.Identities().Create(testcase.CreateIdentity) + if err != nil { + t.Errorf("%s: Could not create identity: %v", k, err) + continue + } + } + if testcase.CreateMapping != nil { + _, err := clusterAdminClient.UserIdentityMappings().Update(testcase.CreateMapping) + if err != nil { + t.Errorf("%s: Could not create mapping: %v", k, err) + continue + } + } + + // Spawn 5 simultaneous mappers to test race conditions + var wg sync.WaitGroup + for i := 0; i < 5; i++ { + wg.Add(1) + go func() { + defer wg.Done() + + userInfo, err := testcase.Mapper.UserFor(testcase.Identity) + if err != nil { + if testcase.ExpectedErr == nil { + t.Errorf("%s: Expected success, got error '%v'", k, err) + } else if err.Error() != testcase.ExpectedErr.Error() { + t.Errorf("%s: Expected error %v, got '%v'", k, testcase.ExpectedErr.Error(), err) + } + return + } + if err == nil && testcase.ExpectedErr != nil { + t.Errorf("%s: Expected error '%v', got none", k, testcase.ExpectedErr) + return + } + + if userInfo.GetName() != testcase.ExpectedUserName { + t.Errorf("%s: Expected username %s, got %s", k, testcase.ExpectedUserName, userInfo.GetName()) + return + } + + user, err := clusterAdminClient.Users().Get(userInfo.GetName()) + if err != nil { + t.Errorf("%s: Error getting user: %v", err) + } + if user.FullName != testcase.ExpectedFullName { + t.Errorf("%s: Expected full name %s, got %s", testcase.ExpectedFullName, user.FullName) + } + + }() + } + wg.Wait() } } diff --git a/test/util/server.go b/test/util/server.go index f899c54237bc..2d6326756134 100644 --- a/test/util/server.go +++ b/test/util/server.go @@ -185,13 +185,12 @@ func StartTestMaster() (*configapi.MasterConfig, string, error) { // CreateNewProject creates a new project using the clusterAdminClient, then gets a token for the adminUser and returns // back a client for the admin user func CreateNewProject(clusterAdminClient *client.Client, clientConfig kclient.Config, projectName, adminUser string) (*client.Client, error) { - qualifiedUser := "anypassword:" + adminUser newProjectOptions := &newproject.NewProjectOptions{ Client: clusterAdminClient, ProjectName: projectName, AdminRole: bootstrappolicy.AdminRoleName, MasterPolicyNamespace: bootstrappolicy.DefaultMasterAuthorizationNamespace, - AdminUser: qualifiedUser, + AdminUser: adminUser, } if err := newProjectOptions.Run(); err != nil {