From 75710d5176ea40362475045443860b72118edb85 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 11 Sep 2015 13:15:27 -0400 Subject: [PATCH] Configurable identity mapper strategies --- .../userregistry/identitymapper/lookup.go | 8 +- .../identitymapper/lookup_test.go | 157 ++++++++ .../userregistry/identitymapper/mapper.go | 59 +++ .../userregistry/identitymapper/provision.go | 105 ++--- .../identitymapper/provision_test.go | 360 ++++++++++++------ .../identitymapper/strategy_add.go | 57 +++ .../identitymapper/strategy_add_test.go | 61 +++ .../identitymapper/strategy_claim.go | 79 ++++ .../identitymapper/strategy_claim_test.go | 61 +++ .../identitymapper/strategy_generate.go | 85 +++++ .../identitymapper/strategy_generate_test.go | 83 ++++ .../identitymapper/strategy_test.go | 110 ++++++ pkg/cmd/server/api/types.go | 2 + pkg/cmd/server/api/v1/conversions.go | 8 + pkg/cmd/server/api/v1/types.go | 2 + pkg/cmd/server/api/v1/types_test.go | 9 + pkg/cmd/server/api/validation/oauth.go | 14 + pkg/cmd/server/origin/auth.go | 15 +- pkg/user/initializer.go | 13 +- pkg/user/initializer_test.go | 52 +++ pkg/user/interfaces.go | 2 + test/integration/auth_proxy_test.go | 5 +- test/integration/cli_get_token_test.go | 5 +- test/integration/oauth_basicauth_test.go | 1 + test/integration/oauth_htpasswd_test.go | 3 +- test/integration/oauth_ldap_test.go | 1 + test/integration/oauth_request_header_test.go | 1 + test/integration/userclient_test.go | 269 +++++++++++-- 28 files changed, 1374 insertions(+), 253 deletions(-) create mode 100644 pkg/auth/userregistry/identitymapper/lookup_test.go create mode 100644 pkg/auth/userregistry/identitymapper/mapper.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_add.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_add_test.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_claim.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_claim_test.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_generate.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_generate_test.go create mode 100644 pkg/auth/userregistry/identitymapper/strategy_test.go create mode 100644 pkg/user/initializer_test.go diff --git a/pkg/auth/userregistry/identitymapper/lookup.go b/pkg/auth/userregistry/identitymapper/lookup.go index fbe1d3dfe7dc..59aae3a1920a 100644 --- a/pkg/auth/userregistry/identitymapper/lookup.go +++ b/pkg/auth/userregistry/identitymapper/lookup.go @@ -9,16 +9,14 @@ import ( "github.com/openshift/origin/pkg/user/registry/useridentitymapping" ) +var _ = authapi.UserIdentityMapper(&lookupIdentityMapper{}) + +// lookupIdentityMapper does not provision a new identity or user, it only allows identities already associated with users type lookupIdentityMapper struct { mappings useridentitymapping.Registry users user.Registry } -// NewLookupIdentityMapper returns a mapper that will look up existing mappings for identities -func NewLookupIdentityMapper(mappings useridentitymapping.Registry, users user.Registry) authapi.UserIdentityMapper { - return &lookupIdentityMapper{mappings, users} -} - // UserFor returns info about the user for whom identity info has been provided func (p *lookupIdentityMapper) UserFor(info authapi.UserIdentityInfo) (kuser.Info, error) { ctx := kapi.NewContext() diff --git a/pkg/auth/userregistry/identitymapper/lookup_test.go b/pkg/auth/userregistry/identitymapper/lookup_test.go new file mode 100644 index 000000000000..a77a3b1e86e5 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/lookup_test.go @@ -0,0 +1,157 @@ +package identitymapper + +import ( + "reflect" + "testing" + + authapi "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/user/api" + userapi "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" + mappingregistry "github.com/openshift/origin/pkg/user/registry/useridentitymapping" +) + +func TestLookup(t *testing.T) { + testcases := map[string]struct { + ProviderName string + ProviderUserName string + + ExistingIdentity *userapi.Identity + ExistingUser *userapi.User + + ExpectedActions []test.Action + ExpectedError bool + ExpectedUserName string + }{ + "no identity": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: nil, + ExistingUser: nil, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + }, + ExpectedError: true, + }, + + "existing identity, no user reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "", ""), + ExistingUser: nil, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + }, + ExpectedError: true, + }, + "existing identity, missing user reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUID", "bob"), + ExistingUser: nil, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedError: true, + }, + "existing identity, invalid user UID reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUIDInvalid", "bob"), + ExistingUser: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedError: true, + }, + "existing identity, user reference without identity backreference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUID", "bob"), + ExistingUser: makeUser("bobUserUID", "bob" /*, "idp:bob"*/), + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedError: true, + }, + "existing identity, user reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUID", "bob"), + ExistingUser: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + {"GetUser", "bob"}, // extra request is for group lookup + }, + ExpectedUserName: "bob", + }, + } + + for k, tc := range testcases { + actions := []test.Action{} + identityRegistry := &test.IdentityRegistry{ + Get: map[string]*api.Identity{}, + Actions: &actions, + } + userRegistry := &test.UserRegistry{ + Get: map[string]*api.User{}, + Actions: &actions, + } + if tc.ExistingIdentity != nil { + identityRegistry.Get[tc.ExistingIdentity.Name] = tc.ExistingIdentity + } + if tc.ExistingUser != nil { + userRegistry.Get[tc.ExistingUser.Name] = tc.ExistingUser + } + + mappingStorage := mappingregistry.NewREST(userRegistry, identityRegistry) + mappingRegistry := mappingregistry.NewRegistry(mappingStorage) + + lookupMapper := &lookupIdentityMapper{ + mappings: mappingRegistry, + users: userRegistry, + } + + identity := authapi.NewDefaultUserIdentityInfo(tc.ProviderName, tc.ProviderUserName) + user, err := lookupMapper.UserFor(identity) + if tc.ExpectedError != (err != nil) { + t.Errorf("%s: Expected error=%v, got %v", k, tc.ExpectedError, err) + continue + } + if !tc.ExpectedError && user.GetName() != tc.ExpectedUserName { + t.Errorf("%s: Expected username %v, got %v", k, tc.ExpectedUserName, user.GetName()) + continue + } + + for i, action := range actions { + if len(tc.ExpectedActions) <= i { + t.Fatalf("%s: expected %d actions, got extras: %#v", k, len(tc.ExpectedActions), actions[i:]) + continue + } + expectedAction := tc.ExpectedActions[i] + if !reflect.DeepEqual(expectedAction, action) { + t.Fatalf("%s: expected\n\t%s %#v\nGot\n\t%s %#v", k, expectedAction.Name, expectedAction.Object, action.Name, action.Object) + continue + } + } + if len(actions) < len(tc.ExpectedActions) { + t.Errorf("Missing %d additional actions:\n\t%#v", len(tc.ExpectedActions)-len(actions), tc.ExpectedActions[len(actions):]) + } + } +} diff --git a/pkg/auth/userregistry/identitymapper/mapper.go b/pkg/auth/userregistry/identitymapper/mapper.go new file mode 100644 index 000000000000..10c0ee6816d5 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/mapper.go @@ -0,0 +1,59 @@ +package identitymapper + +import ( + "fmt" + + authapi "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/user" + identityregistry "github.com/openshift/origin/pkg/user/registry/identity" + userregistry "github.com/openshift/origin/pkg/user/registry/user" + mappingregistry "github.com/openshift/origin/pkg/user/registry/useridentitymapping" +) + +type MappingMethodType string + +const ( + // MappingMethodLookup does not provision a new identity or user, it only allows identities already associated with users + MappingMethodLookup MappingMethodType = "lookup" + + // MappingMethodClaim associates a new identity with a user with the identity's preferred username + // if no other identities are already associated with the user + MappingMethodClaim MappingMethodType = "claim" + + // MappingMethodAdd associates a new identity with a user with the identity's preferred username, + // creating the user if needed, and adding to any existing identities associated with the user + MappingMethodAdd MappingMethodType = "add" + + // MappingMethodGenerate finds an available username for a new identity, based on its preferred username + // If a user with the preferred username already exists, a unique username is generated + MappingMethodGenerate MappingMethodType = "generate" +) + +// NewIdentityUserMapper returns a UserIdentityMapper 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 (or is associated with a missing user) +// 3. Handles new identities according to the requested method +func NewIdentityUserMapper(identities identityregistry.Registry, users userregistry.Registry, method MappingMethodType) (authapi.UserIdentityMapper, error) { + // initUser initializes fields in a User API object from its associated Identity + // called when adding the first Identity to a User (during create or update of a User) + initUser := user.NewDefaultUserInitStrategy() + + switch method { + case MappingMethodLookup: + mappingStorage := mappingregistry.NewREST(users, identities) + mappingRegistry := mappingregistry.NewRegistry(mappingStorage) + return &lookupIdentityMapper{mappingRegistry, users}, nil + + case MappingMethodClaim: + return &provisioningIdentityMapper{identities, users, NewStrategyClaim(users, initUser)}, nil + + case MappingMethodAdd: + return &provisioningIdentityMapper{identities, users, NewStrategyAdd(users, initUser)}, nil + + case MappingMethodGenerate: + return &provisioningIdentityMapper{identities, users, NewStrategyGenerate(users, initUser)}, nil + + default: + return nil, fmt.Errorf("unsupported mapping method %q", method) + } +} diff --git a/pkg/auth/userregistry/identitymapper/provision.go b/pkg/auth/userregistry/identitymapper/provision.go index e9a1e23bf803..124d58dee4c2 100644 --- a/pkg/auth/userregistry/identitymapper/provision.go +++ b/pkg/auth/userregistry/identitymapper/provision.go @@ -1,9 +1,6 @@ package identitymapper import ( - "errors" - "fmt" - "github.com/golang/glog" kapi "k8s.io/kubernetes/pkg/api" kerrs "k8s.io/kubernetes/pkg/api/errors" @@ -11,41 +8,28 @@ import ( "k8s.io/kubernetes/pkg/util/sets" 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 +// UserForNewIdentityGetter is responsible for creating or locating the persisted User for the given Identity. +// The preferredUserName is available to the strategies +type UserForNewIdentityGetter interface { + // UserForNewIdentity returns a persisted User object for the given Identity, creating it if needed + UserForNewIdentity(ctx kapi.Context, preferredUserName string, identity *userapi.Identity) (*userapi.User, error) +} - // 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) - }) -) +var _ = authapi.UserIdentityMapper(&provisioningIdentityMapper{}) +// provisioningIdentityMapper implements api.UserIdentityMapper +// If an existing UserIdentityMapping exists for an identity, it is returned. +// If an identity does not exist, it creates an Identity referencing the user returned from provisioningStrategy.UserForNewIdentity +// Otherwise an error is returned 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()} + identity identityregistry.Registry + user userregistry.Registry + provisioningStrategy UserForNewIdentityGetter } // UserFor returns info about the user for whom identity info have been provided @@ -67,11 +51,14 @@ func (p *provisioningIdentityMapper) userForWithRetries(info authapi.UserIdentit if kerrs.IsNotFound(err) { user, err := p.createIdentityAndMapping(ctx, info) - // Only retry for AlreadyExists errors, which can occur in the following cases: + // Only retry for the following types of errors: + // AlreadyExists errors: // * 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 { + // * The same user was created by another instance of this identity provider (e.g. double-clicked login button) + // * The same identity was created by another instance of this identity provider (e.g. double-clicked login button) + // Conflict errors: + // * The same user was updated be another identity provider to add identity info + if (kerrs.IsAlreadyExists(err) || kerrs.IsConflict(err)) && allowedRetries > 0 { return p.userForWithRetries(info, allowedRetries-1) } return user, err @@ -97,7 +84,7 @@ func (p *provisioningIdentityMapper) createIdentityAndMapping(ctx kapi.Context, } // Get or create a persisted user pointing to the identity - persistedUser, err := p.getOrCreateUserForIdentity(ctx, identity) + persistedUser, err := p.provisioningStrategy.UserForNewIdentity(ctx, getPreferredUserName(identity), identity) if err != nil { return nil, err } @@ -118,54 +105,6 @@ func (p *provisioningIdentityMapper) createIdentityAndMapping(ctx kapi.Context, }, 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 sets.NewString(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) diff --git a/pkg/auth/userregistry/identitymapper/provision_test.go b/pkg/auth/userregistry/identitymapper/provision_test.go index 0257048b3a27..ec178f67d805 100644 --- a/pkg/auth/userregistry/identitymapper/provision_test.go +++ b/pkg/auth/userregistry/identitymapper/provision_test.go @@ -1,17 +1,39 @@ package identitymapper import ( + "fmt" "reflect" "testing" kapi "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/types" + kerrs "k8s.io/kubernetes/pkg/api/errors" authapi "github.com/openshift/origin/pkg/auth/api" "github.com/openshift/origin/pkg/user/api" + userapi "github.com/openshift/origin/pkg/user/api" "github.com/openshift/origin/pkg/user/registry/test" ) +type testNewIdentityGetter struct { + called int + responses []interface{} +} + +func (t *testNewIdentityGetter) UserForNewIdentity(ctx kapi.Context, preferredUserName string, identity *userapi.Identity) (*userapi.User, error) { + t.called++ + if len(t.responses) < t.called { + return nil, fmt.Errorf("Called at least %d times, only %d responses registered", t.called, len(t.responses)) + } + switch response := t.responses[t.called-1].(type) { + case error: + return nil, response + case *userapi.User: + return response, nil + default: + return nil, fmt.Errorf("Invalid response type registered: %#v", response) + } +} + func TestGetPreferredUsername(t *testing.T) { identity := &api.Identity{} @@ -26,142 +48,246 @@ func TestGetPreferredUsername(t *testing.T) { } } -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), +func TestProvision(t *testing.T) { + testcases := map[string]struct { + ProviderName string + ProviderUserName string + + ExistingIdentity *userapi.Identity + ExistingUser *userapi.User + NewIdentityGetterResponses []interface{} + + ExpectedActions []test.Action + ExpectedError bool + ExpectedUserName string + }{ + "no identity, create user succeeds": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: nil, + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{ + makeUser("bobUserUID", "bob", "idp:bob"), + }, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + // ... new identity user getter creates user + {"CreateIdentity", makeIdentity("", "idp", "bob", "bobUserUID", "bob")}, + }, + ExpectedUserName: "bob", }, - 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, + "no identity, alreadyexists error retries": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: nil, + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{ + kerrs.NewAlreadyExists("User", "bob"), + makeUser("bobUserUID", "bob", "idp:bob"), + }, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + {"GetIdentity", "idp:bob"}, + // ... new identity user getter creates user + {"CreateIdentity", makeIdentity("", "idp", "bob", "bobUserUID", "bob")}, + }, + ExpectedUserName: "bob", }, - Identities: []string{expectedIdentityName}, - } - // Return a user containing a uid - expectedCreateUserResult := &api.User{ - ObjectMeta: kapi.ObjectMeta{ - Name: expectedUserName, - UID: types.UID(expectedUserUID), + "no identity, conflict error retries": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: nil, + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{ + kerrs.NewConflict("User", "bob", fmt.Errorf("conflict")), + makeUser("bobUserUID", "bob", "idp:bob"), + }, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + {"GetIdentity", "idp:bob"}, + // ... new identity user getter creates user + {"CreateIdentity", makeIdentity("", "idp", "bob", "bobUserUID", "bob")}, + }, + ExpectedUserName: "bob", }, - Identities: []string{expectedIdentityName}, - } + "no identity, only retries 3 times": { + ProviderName: "idp", + ProviderUserName: "bob", - expectedActions := []test.Action{ - {"GetIdentity", expectedIdentityName}, - {"GetUser", expectedProviderUserName}, - {"CreateUser", expectedCreateUser}, - {"CreateIdentity", expectedCreateIdentity}, - } + ExistingIdentity: nil, + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{ + kerrs.NewConflict("User", "bob", fmt.Errorf("conflict")), + kerrs.NewConflict("User", "bob", fmt.Errorf("conflict")), + kerrs.NewConflict("User", "bob", fmt.Errorf("conflict")), + kerrs.NewConflict("User", "bob", fmt.Errorf("conflict")), + }, - actions := []test.Action{} - identityRegistry := &test.IdentityRegistry{ - Create: expectedCreateIdentity, - Actions: &actions, - } - userRegistry := &test.UserRegistry{ - Create: expectedCreateUserResult, - Actions: &actions, - } + ExpectedActions: []test.Action{ + // original attempt + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + // retry #1 + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + // retry #2 + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + // retry #3 + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + }, + ExpectedError: true, + }, + "no identity, unknown error does not retry": { + ProviderName: "idp", + ProviderUserName: "bob", - identityMapper := NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) - identity := authapi.NewDefaultUserIdentityInfo(expectedProviderName, expectedProviderUserName) + ExistingIdentity: nil, + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{ + fmt.Errorf("other error"), + }, - identityMapper.UserFor(identity) + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + // ... new identity user getter returns error + }, + ExpectedError: true, + }, - 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) - } - } -} + "existing identity, no user reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "", ""), + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{}, -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), + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + }, + ExpectedError: true, }, - 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, + "existing identity, missing user reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUID", "bob"), + ExistingUser: nil, + NewIdentityGetterResponses: []interface{}{}, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedError: true, }, - Identities: []string{expectedIdentityName}, - } - // Return a user containing a uid - expectedCreateUserResult := &api.User{ - ObjectMeta: kapi.ObjectMeta{ - Name: expectedUserName, - UID: types.UID(expectedUserUID), + "existing identity, invalid user UID reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUIDInvalid", "bob"), + ExistingUser: makeUser("bobUserUID", "bob", "idp:bob"), + NewIdentityGetterResponses: []interface{}{}, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedError: true, }, - Identities: []string{expectedIdentityName}, - } + "existing identity, user reference without identity backreference": { + ProviderName: "idp", + ProviderUserName: "bob", - expectedActions := []test.Action{ - {"GetIdentity", expectedIdentityName}, - {"GetUser", expectedProviderUserName}, - {"GetUser", expectedProviderUserName + "2"}, - {"GetUser", expectedProviderUserName + "3"}, - {"CreateUser", expectedCreateUser}, - {"CreateIdentity", expectedCreateIdentity}, - } + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUID", "bob"), + ExistingUser: makeUser("bobUserUID", "bob" /*, "idp:bob"*/), + NewIdentityGetterResponses: []interface{}{}, - actions := []test.Action{} - identityRegistry := &test.IdentityRegistry{ - Create: expectedCreateIdentity, - Actions: &actions, - } - userRegistry := &test.UserRegistry{ - Get: map[string]*api.User{ - expectedProviderUserName: {}, - expectedProviderUserName + "2": {}, + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedError: true, + }, + "existing identity, user reference": { + ProviderName: "idp", + ProviderUserName: "bob", + + ExistingIdentity: makeIdentity("bobIdentityUID", "idp", "bob", "bobUserUID", "bob"), + ExistingUser: makeUser("bobUserUID", "bob", "idp:bob"), + NewIdentityGetterResponses: []interface{}{}, + + ExpectedActions: []test.Action{ + {"GetIdentity", "idp:bob"}, + {"GetUser", "bob"}, + }, + ExpectedUserName: "bob", }, - Create: expectedCreateUserResult, - Actions: &actions, } - identityMapper := NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) - identity := authapi.NewDefaultUserIdentityInfo(expectedProviderName, expectedProviderUserName) + for k, tc := range testcases { + actions := []test.Action{} + identityRegistry := &test.IdentityRegistry{ + Get: map[string]*api.Identity{}, + Actions: &actions, + } + userRegistry := &test.UserRegistry{ + Get: map[string]*api.User{}, + Actions: &actions, + } + if tc.ExistingIdentity != nil { + identityRegistry.Get[tc.ExistingIdentity.Name] = tc.ExistingIdentity + } + if tc.ExistingUser != nil { + userRegistry.Get[tc.ExistingUser.Name] = tc.ExistingUser + } + + newIdentityUserGetter := &testNewIdentityGetter{responses: tc.NewIdentityGetterResponses} - identityMapper.UserFor(identity) + provisionMapper := &provisioningIdentityMapper{ + identity: identityRegistry, + user: userRegistry, + provisioningStrategy: newIdentityUserGetter, + } + + identity := authapi.NewDefaultUserIdentityInfo(tc.ProviderName, tc.ProviderUserName) + user, err := provisionMapper.UserFor(identity) + if tc.ExpectedError != (err != nil) { + t.Errorf("%s: Expected error=%v, got %v", k, tc.ExpectedError, err) + continue + } + if !tc.ExpectedError && user.GetName() != tc.ExpectedUserName { + t.Errorf("%s: Expected username %v, got %v", k, tc.ExpectedUserName, user.GetName()) + continue + } + + if newIdentityUserGetter.called != len(tc.NewIdentityGetterResponses) { + t.Errorf("%s: Expected %d calls to UserForNewIdentity, got %d", k, len(tc.NewIdentityGetterResponses), newIdentityUserGetter.called) + } - for i, action := range actions { - if len(expectedActions) <= i { - t.Fatalf("Expected %d actions, got extras: %#v", len(expectedActions), actions[i:]) + for i, action := range actions { + if len(tc.ExpectedActions) <= i { + t.Fatalf("%s: expected %d actions, got extras: %#v", k, len(tc.ExpectedActions), actions[i:]) + continue + } + expectedAction := tc.ExpectedActions[i] + if !reflect.DeepEqual(expectedAction, action) { + t.Fatalf("%s: expected\n\t%s %#v\nGot\n\t%s %#v", k, expectedAction.Name, expectedAction.Object, action.Name, action.Object) + continue + } } - 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) + if len(actions) < len(tc.ExpectedActions) { + t.Errorf("Missing %d additional actions:\n\t%#v", len(tc.ExpectedActions)-len(actions), tc.ExpectedActions[len(actions):]) } } } diff --git a/pkg/auth/userregistry/identitymapper/strategy_add.go b/pkg/auth/userregistry/identitymapper/strategy_add.go new file mode 100644 index 000000000000..244e6fba8228 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_add.go @@ -0,0 +1,57 @@ +package identitymapper + +import ( + kapi "k8s.io/kubernetes/pkg/api" + kerrs "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/util/sets" + + "github.com/openshift/origin/pkg/user" + userapi "github.com/openshift/origin/pkg/user/api" + userregistry "github.com/openshift/origin/pkg/user/registry/user" +) + +var _ = UserForNewIdentityGetter(&StrategyAdd{}) + +// StrategyAdd associates a new identity with a user with the identity's preferred username, +// adding to any existing identities associated with the user +type StrategyAdd struct { + user userregistry.Registry + initializer user.Initializer +} + +func NewStrategyAdd(user userregistry.Registry, initializer user.Initializer) UserForNewIdentityGetter { + return &StrategyAdd{user, initializer} +} + +func (s *StrategyAdd) UserForNewIdentity(ctx kapi.Context, preferredUserName string, identity *userapi.Identity) (*userapi.User, error) { + + persistedUser, err := s.user.GetUser(ctx, preferredUserName) + + switch { + case kerrs.IsNotFound(err): + // Create a new user + desiredUser := &userapi.User{} + desiredUser.Name = preferredUserName + desiredUser.Identities = []string{identity.Name} + s.initializer.InitializeUser(identity, desiredUser) + return s.user.CreateUser(ctx, desiredUser) + + case err == nil: + // If the existing user already references our identity, we're done + if sets.NewString(persistedUser.Identities...).Has(identity.Name) { + return persistedUser, nil + } + + // Otherwise add our identity and update + persistedUser.Identities = append(persistedUser.Identities, identity.Name) + // If our newly added identity is the only one, initialize the user + if len(persistedUser.Identities) == 1 { + s.initializer.InitializeUser(identity, persistedUser) + } + return s.user.UpdateUser(ctx, persistedUser) + + default: + // Fail on errors other than "not found" + return nil, err + } +} diff --git a/pkg/auth/userregistry/identitymapper/strategy_add_test.go b/pkg/auth/userregistry/identitymapper/strategy_add_test.go new file mode 100644 index 000000000000..5df10845bf62 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_add_test.go @@ -0,0 +1,61 @@ +package identitymapper + +import ( + "testing" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" +) + +func TestStrategyAdd(t *testing.T) { + testcases := map[string]strategyTestCase{ + "no user": { + MakeStrategy: NewStrategyAdd, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + CreateResponse: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"CreateUser", makeUser("", "bob", "idp:bob")}, + }, + ExpectedUserName: "bob", + ExpectedInitialize: true, + }, + "existing user, no identities": { + MakeStrategy: NewStrategyAdd, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{makeUser("bobUserUID", "bob")}, + UpdateResponse: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"UpdateUser", makeUser("bobUserUID", "bob", "idp:bob")}, + }, + ExpectedUserName: "bob", + ExpectedInitialize: true, + }, + "existing user, conflicting identity": { + MakeStrategy: NewStrategyAdd, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{makeUser("bobUserUID", "bob", "otheridp:user")}, + UpdateResponse: makeUser("bobUserUID", "bob", "otheridp:user", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"UpdateUser", makeUser("bobUserUID", "bob", "otheridp:user", "idp:bob")}, + }, + ExpectedUserName: "bob", + ExpectedInitialize: false, + }, + } + + for testCaseName, testCase := range testcases { + testCase.run(testCaseName, t) + } +} diff --git a/pkg/auth/userregistry/identitymapper/strategy_claim.go b/pkg/auth/userregistry/identitymapper/strategy_claim.go new file mode 100644 index 000000000000..e300f67a1720 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_claim.go @@ -0,0 +1,79 @@ +package identitymapper + +import ( + "fmt" + + kapi "k8s.io/kubernetes/pkg/api" + kerrs "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/util/sets" + + "github.com/openshift/origin/pkg/user" + userapi "github.com/openshift/origin/pkg/user/api" + userregistry "github.com/openshift/origin/pkg/user/registry/user" +) + +var _ = UserForNewIdentityGetter(&StrategyClaim{}) + +// StrategyClaim associates a new identity with a user with the identity's preferred username +// if no other identities are already associated with the user +type StrategyClaim struct { + user userregistry.Registry + initializer user.Initializer +} + +type claimError struct { + User *userapi.User + Identity *userapi.Identity +} + +func IsClaimError(err error) bool { + _, ok := err.(claimError) + return ok +} +func NewClaimError(user *userapi.User, identity *userapi.Identity) error { + return claimError{user, identity} +} + +func (c claimError) Error() string { + return fmt.Sprintf("user %q cannot be claimed by identity %q because it is already mapped to %v", c.User.Name, c.Identity.Name, c.User.Identities) +} + +func NewStrategyClaim(user userregistry.Registry, initializer user.Initializer) UserForNewIdentityGetter { + return &StrategyClaim{user, initializer} +} + +func (s *StrategyClaim) UserForNewIdentity(ctx kapi.Context, preferredUserName string, identity *userapi.Identity) (*userapi.User, error) { + + persistedUser, err := s.user.GetUser(ctx, preferredUserName) + + switch { + case kerrs.IsNotFound(err): + // Create a new user, propagating any "already exists" errors + desiredUser := &userapi.User{} + desiredUser.Name = preferredUserName + desiredUser.Identities = []string{identity.Name} + s.initializer.InitializeUser(identity, desiredUser) + return s.user.CreateUser(ctx, desiredUser) + + case err == nil: + // If the existing user already references our identity, we're done + if sets.NewString(persistedUser.Identities...).Has(identity.Name) { + return persistedUser, nil + } + + // If this user has no other identities, claim, initialize, and update + if len(persistedUser.Identities) == 0 { + persistedUser.Identities = []string{identity.Name} + s.initializer.InitializeUser(identity, persistedUser) + return s.user.UpdateUser(ctx, persistedUser) + } + + // Otherwise another identity has already claimed this user, return an error + return nil, NewClaimError(persistedUser, identity) + + default: + // Fail on errors other than "not found" + return nil, err + } + +} diff --git a/pkg/auth/userregistry/identitymapper/strategy_claim_test.go b/pkg/auth/userregistry/identitymapper/strategy_claim_test.go new file mode 100644 index 000000000000..2ff3e0ec28c7 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_claim_test.go @@ -0,0 +1,61 @@ +package identitymapper + +import ( + "testing" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" +) + +func TestStrategyClaim(t *testing.T) { + testcases := map[string]strategyTestCase{ + "no user": { + MakeStrategy: NewStrategyClaim, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + CreateResponse: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"CreateUser", makeUser("", "bob", "idp:bob")}, + }, + ExpectedUserName: "bob", + ExpectedInitialize: true, + }, + "existing user, no identities": { + MakeStrategy: NewStrategyClaim, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{makeUser("bobUserUID", "bob")}, + UpdateResponse: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"UpdateUser", makeUser("bobUserUID", "bob", "idp:bob")}, + }, + ExpectedUserName: "bob", + ExpectedInitialize: true, + }, + "existing user, conflicting identity": { + MakeStrategy: NewStrategyClaim, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{makeUser("bobUserUID", "bob", "otheridp:user")}, + UpdateResponse: makeUser("bobUserUID", "bob", "otheridp:user", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + }, + ExpectedError: true, + ExpectedUserName: "", + ExpectedInitialize: false, + }, + } + + for testCaseName, testCase := range testcases { + testCase.run(testCaseName, t) + } +} diff --git a/pkg/auth/userregistry/identitymapper/strategy_generate.go b/pkg/auth/userregistry/identitymapper/strategy_generate.go new file mode 100644 index 000000000000..8d9cf1cef28b --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_generate.go @@ -0,0 +1,85 @@ +package identitymapper + +import ( + "errors" + "fmt" + + kapi "k8s.io/kubernetes/pkg/api" + kerrs "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/util/sets" + + "github.com/openshift/origin/pkg/user" + userapi "github.com/openshift/origin/pkg/user/api" + 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) + }) +) + +var _ = UserForNewIdentityGetter(&StrategyGenerate{}) + +// StrategyGenerate finds an available username for a new identity, based on its preferred username +// If a user with the preferred username already exists, a unique username is generated +type StrategyGenerate struct { + user userregistry.Registry + generator UserNameGenerator + initializer user.Initializer +} + +func NewStrategyGenerate(user userregistry.Registry, initializer user.Initializer) UserForNewIdentityGetter { + return &StrategyGenerate{user, DefaultGenerator, initializer} +} + +func (s *StrategyGenerate) UserForNewIdentity(ctx kapi.Context, preferredUserName string, identity *userapi.Identity) (*userapi.User, error) { + + // 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. +UserSearch: + for sequence := 0; sequence < MaxGenerateAttempts; sequence++ { + // Get the username we want + potentialUserName := s.generator(preferredUserName, sequence) + + // See if it already exists + persistedUser, err := s.user.GetUser(ctx, potentialUserName) + + switch { + case kerrs.IsNotFound(err): + // Create a new user + desiredUser := &userapi.User{} + desiredUser.Name = potentialUserName + desiredUser.Identities = []string{identity.Name} + s.initializer.InitializeUser(identity, desiredUser) + return s.user.CreateUser(ctx, desiredUser) + + case err == nil: + // If the existing user already references our identity, we're done + if sets.NewString(persistedUser.Identities...).Has(identity.Name) { + return persistedUser, nil + } + // Otherwise, continue our search for a user + continue UserSearch + + default: + // Fail on errors other than "not found" + return nil, err + } + } + + return nil, errors.New("Could not create user, max attempts exceeded") +} diff --git a/pkg/auth/userregistry/identitymapper/strategy_generate_test.go b/pkg/auth/userregistry/identitymapper/strategy_generate_test.go new file mode 100644 index 000000000000..6af6c8fb281b --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_generate_test.go @@ -0,0 +1,83 @@ +package identitymapper + +import ( + "testing" + + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" +) + +func TestStrategyGenerate(t *testing.T) { + testcases := map[string]strategyTestCase{ + "no user": { + MakeStrategy: NewStrategyGenerate, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + CreateResponse: makeUser("bobUserUID", "bob", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"CreateUser", makeUser("", "bob", "idp:bob")}, + }, + ExpectedUserName: "bob", + ExpectedInitialize: true, + }, + "existing user, no identities": { + MakeStrategy: NewStrategyGenerate, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{makeUser("bobUserUID", "bob")}, + CreateResponse: makeUser("bob2UserUID", "bob2", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"GetUser", "bob2"}, + {"CreateUser", makeUser("", "bob2", "idp:bob")}, + }, + ExpectedUserName: "bob2", + ExpectedInitialize: true, + }, + "existing user, conflicting identity": { + MakeStrategy: NewStrategyGenerate, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{makeUser("bobUserUID", "bob", "otheridp:user")}, + CreateResponse: makeUser("bob2UserUID", "bob2", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"GetUser", "bob2"}, + {"CreateUser", makeUser("", "bob2", "idp:bob")}, + }, + ExpectedUserName: "bob2", + ExpectedInitialize: true, + }, + "existing users": { + MakeStrategy: NewStrategyGenerate, + PreferredUsername: "bob", + Identity: makeIdentity("", "idp", "bob", "", ""), + + ExistingUsers: []*api.User{ + makeUser("bobUserUID", "bob", "otheridp:user"), + makeUser("bob2UserUID", "bob2", "otheridp:user2"), + }, + CreateResponse: makeUser("bob3UserUID", "bob3", "idp:bob"), + + ExpectedActions: []test.Action{ + {"GetUser", "bob"}, + {"GetUser", "bob2"}, + {"GetUser", "bob3"}, + {"CreateUser", makeUser("", "bob3", "idp:bob")}, + }, + ExpectedUserName: "bob3", + ExpectedInitialize: true, + }, + } + + for testCaseName, testCase := range testcases { + testCase.run(testCaseName, t) + } +} diff --git a/pkg/auth/userregistry/identitymapper/strategy_test.go b/pkg/auth/userregistry/identitymapper/strategy_test.go new file mode 100644 index 000000000000..060b69e365b2 --- /dev/null +++ b/pkg/auth/userregistry/identitymapper/strategy_test.go @@ -0,0 +1,110 @@ +package identitymapper + +import ( + "reflect" + "testing" + + kapi "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/types" + + "github.com/openshift/origin/pkg/user" + "github.com/openshift/origin/pkg/user/api" + "github.com/openshift/origin/pkg/user/registry/test" + userregistry "github.com/openshift/origin/pkg/user/registry/user" +) + +type testInitializer struct { + called bool +} + +func (t *testInitializer) InitializeUser(identity *api.Identity, user *api.User) error { + t.called = true + return nil +} + +type strategyTestCase struct { + MakeStrategy func(user userregistry.Registry, initializer user.Initializer) UserForNewIdentityGetter + + // Inputs + PreferredUsername string + Identity *api.Identity + + // User registry setup + ExistingUsers []*api.User + CreateResponse *api.User + UpdateResponse *api.User + + // Expectations + ExpectedActions []test.Action + ExpectedError bool + ExpectedUserName string + ExpectedInitialize bool +} + +func makeUser(uid string, name string, identities ...string) *api.User { + return &api.User{ + ObjectMeta: kapi.ObjectMeta{ + Name: name, + UID: types.UID(uid), + }, + Identities: identities, + } +} +func makeIdentity(uid string, providerName string, providerUserName string, userUID string, userName string) *api.Identity { + return &api.Identity{ + ObjectMeta: kapi.ObjectMeta{ + Name: providerName + ":" + providerUserName, + UID: types.UID(uid), + }, + ProviderName: providerName, + ProviderUserName: providerUserName, + User: kapi.ObjectReference{ + UID: types.UID(userUID), + Name: userName, + }, + Extra: map[string]string{}, + } +} + +func (tc strategyTestCase) run(k string, t *testing.T) { + actions := []test.Action{} + userRegistry := &test.UserRegistry{ + Get: map[string]*api.User{}, + Actions: &actions, + } + for _, u := range tc.ExistingUsers { + userRegistry.Get[u.Name] = u + } + + testInit := &testInitializer{} + strategy := tc.MakeStrategy(userRegistry, testInit) + + user, err := strategy.UserForNewIdentity(kapi.NewContext(), tc.PreferredUsername, tc.Identity) + if tc.ExpectedError != (err != nil) { + t.Errorf("%s: Expected error=%v, got %v", k, tc.ExpectedError, err) + return + } + if !tc.ExpectedError && user.Name != tc.ExpectedUserName { + t.Errorf("%s: Expected username %v, got %v", k, tc.ExpectedUserName, user.Name) + return + } + + if tc.ExpectedInitialize != testInit.called { + t.Errorf("%s: Expected initialize=%v, got initialize=%v", k, tc.ExpectedInitialize, testInit.called) + } + + for i, action := range actions { + if len(tc.ExpectedActions) <= i { + t.Errorf("%s: expected %d actions, got extras: %#v", k, len(tc.ExpectedActions), actions[i:]) + return + } + expectedAction := tc.ExpectedActions[i] + if !reflect.DeepEqual(expectedAction, action) { + t.Errorf("%s: expected\n\t%s %#v\nGot\n\t%s %#v", k, expectedAction.Name, expectedAction.Object, action.Name, action.Object) + continue + } + } + if len(actions) < len(tc.ExpectedActions) { + t.Errorf("Missing %d additional actions:\n\t%#v", len(tc.ExpectedActions)-len(actions), tc.ExpectedActions[len(actions):]) + } +} diff --git a/pkg/cmd/server/api/types.go b/pkg/cmd/server/api/types.go index 1dbb2a9bd804..68b52eaf8f75 100644 --- a/pkg/cmd/server/api/types.go +++ b/pkg/cmd/server/api/types.go @@ -514,6 +514,8 @@ type IdentityProvider struct { UseAsChallenger bool // UseAsLogin indicates whether to use this identity provider for unauthenticated browsers to login against UseAsLogin bool + // MappingMethod determines how identities from this provider are mapped to users + MappingMethod string // Provider contains the information about how to set up a specific identity provider Provider runtime.EmbeddedObject } diff --git a/pkg/cmd/server/api/v1/conversions.go b/pkg/cmd/server/api/v1/conversions.go index c0a2cca2c99a..49da65ccdefb 100644 --- a/pkg/cmd/server/api/v1/conversions.go +++ b/pkg/cmd/server/api/v1/conversions.go @@ -128,6 +128,14 @@ func init() { obj.MCSLabelsPerProject = 5 } }, + func(obj *IdentityProvider) { + if len(obj.MappingMethod) == 0 { + // By default, only let one identity provider authenticate a particular user + // If multiple identity providers collide, the second one in will fail to auth + // The admin can set this to "add" if they want to allow new identities to join existing users + obj.MappingMethod = "claim" + } + }, ) if err != nil { // If one of the conversion functions is malformed, detect it immediately. diff --git a/pkg/cmd/server/api/v1/types.go b/pkg/cmd/server/api/v1/types.go index aaf2666bcb87..edafdc210d64 100644 --- a/pkg/cmd/server/api/v1/types.go +++ b/pkg/cmd/server/api/v1/types.go @@ -493,6 +493,8 @@ type IdentityProvider struct { UseAsChallenger bool `json:"challenge"` // UseAsLogin indicates whether to use this identity provider for unauthenticated browsers to login against UseAsLogin bool `json:"login"` + // MappingMethod determines how identities from this provider are mapped to users + MappingMethod string `json:"mappingMethod"` // Provider contains the information about how to set up a specific identity provider Provider runtime.RawExtension `json:"provider"` } diff --git a/pkg/cmd/server/api/v1/types_test.go b/pkg/cmd/server/api/v1/types_test.go index a270057ba561..c8796d9d01b2 100644 --- a/pkg/cmd/server/api/v1/types_test.go +++ b/pkg/cmd/server/api/v1/types_test.go @@ -153,6 +153,7 @@ oauthConfig: identityProviders: - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 @@ -163,18 +164,21 @@ oauthConfig: url: "" - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 kind: AllowAllPasswordIdentityProvider - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 kind: DenyAllPasswordIdentityProvider - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 @@ -182,6 +186,7 @@ oauthConfig: kind: HTPasswdPasswordIdentityProvider - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 @@ -198,6 +203,7 @@ oauthConfig: url: "" - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 @@ -208,6 +214,7 @@ oauthConfig: loginURL: "" - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 @@ -216,6 +223,7 @@ oauthConfig: kind: GitHubIdentityProvider - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 @@ -225,6 +233,7 @@ oauthConfig: kind: GoogleIdentityProvider - challenge: false login: false + mappingMethod: "" name: "" provider: apiVersion: v1 diff --git a/pkg/cmd/server/api/validation/oauth.go b/pkg/cmd/server/api/validation/oauth.go index 04d3a9169d9b..5791e4bda66b 100644 --- a/pkg/cmd/server/api/validation/oauth.go +++ b/pkg/cmd/server/api/validation/oauth.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/origin/pkg/auth/authenticator/redirector" "github.com/openshift/origin/pkg/auth/server/login" + "github.com/openshift/origin/pkg/auth/userregistry/identitymapper" "github.com/openshift/origin/pkg/cmd/server/api" "github.com/openshift/origin/pkg/cmd/server/api/latest" "github.com/openshift/origin/pkg/user/api/validation" @@ -110,6 +111,13 @@ func ValidateOAuthConfig(config *api.OAuthConfig) ValidationResults { return validationResults } +var validMappingMethods = sets.NewString( + string(identitymapper.MappingMethodLookup), + string(identitymapper.MappingMethodClaim), + string(identitymapper.MappingMethodAdd), + string(identitymapper.MappingMethodGenerate), +) + func ValidateIdentityProvider(identityProvider api.IdentityProvider) ValidationResults { validationResults := ValidationResults{} @@ -120,6 +128,12 @@ func ValidateIdentityProvider(identityProvider api.IdentityProvider) ValidationR validationResults.AddErrors(fielderrors.NewFieldInvalid("name", identityProvider.Name, err)) } + if len(identityProvider.MappingMethod) == 0 { + validationResults.AddErrors(fielderrors.NewFieldRequired("mappingMethod")) + } else if !validMappingMethods.Has(identityProvider.MappingMethod) { + validationResults.AddErrors(fielderrors.NewFieldValueNotSupported("mappingMethod", identityProvider.MappingMethod, validMappingMethods.List())) + } + if !api.IsIdentityProviderType(identityProvider.Provider) { validationResults.AddErrors(fielderrors.NewFieldInvalid("provider", identityProvider.Provider, fmt.Sprintf("%v is invalid in this context", identityProvider.Provider))) } else { diff --git a/pkg/cmd/server/origin/auth.go b/pkg/cmd/server/origin/auth.go index b35221b17c6a..871033007639 100644 --- a/pkg/cmd/server/origin/auth.go +++ b/pkg/cmd/server/origin/auth.go @@ -327,7 +327,10 @@ func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, errorHandler hand redirectors := map[string]handlers.AuthenticationRedirector{} for _, identityProvider := range c.Options.IdentityProviders { - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(c.IdentityRegistry, c.UserRegistry) + identityMapper, err := identitymapper.NewIdentityUserMapper(c.IdentityRegistry, c.UserRegistry, identitymapper.MappingMethodType(identityProvider.MappingMethod)) + if err != nil { + return nil, err + } // TODO: refactor handler building per type if configapi.IsPasswordAuthenticator(identityProvider) { @@ -467,7 +470,10 @@ func (c *AuthConfig) getOAuthProvider(identityProvider configapi.IdentityProvide } func (c *AuthConfig) getPasswordAuthenticator(identityProvider configapi.IdentityProvider) (authenticator.Password, error) { - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(c.IdentityRegistry, c.UserRegistry) + identityMapper, err := identitymapper.NewIdentityUserMapper(c.IdentityRegistry, c.UserRegistry, identitymapper.MappingMethodType(identityProvider.MappingMethod)) + if err != nil { + return nil, err + } switch provider := identityProvider.Provider.Object.(type) { case (*configapi.AllowAllPasswordIdentityProvider): @@ -534,7 +540,10 @@ func (c *AuthConfig) getAuthenticationRequestHandler() (authenticator.Request, e } for _, identityProvider := range c.Options.IdentityProviders { - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(c.IdentityRegistry, c.UserRegistry) + identityMapper, err := identitymapper.NewIdentityUserMapper(c.IdentityRegistry, c.UserRegistry, identitymapper.MappingMethodType(identityProvider.MappingMethod)) + if err != nil { + return nil, err + } if configapi.IsPasswordAuthenticator(identityProvider) { passwordAuthenticator, err := c.getPasswordAuthenticator(identityProvider) diff --git a/pkg/user/initializer.go b/pkg/user/initializer.go index a37c127f2e03..3aacf9766c0d 100644 --- a/pkg/user/initializer.go +++ b/pkg/user/initializer.go @@ -1,6 +1,9 @@ package user -import "github.com/openshift/origin/pkg/user/api" +import ( + authapi "github.com/openshift/origin/pkg/auth/api" + userapi "github.com/openshift/origin/pkg/user/api" +) type DefaultUserInitStrategy struct { } @@ -10,11 +13,9 @@ func NewDefaultUserInitStrategy() Initializer { } // InitializeUser implements Initializer -func (*DefaultUserInitStrategy) InitializeUser(identity *api.Identity, user *api.User) error { - if identity.Extra != nil { - if name, ok := identity.Extra["name"]; ok && len(name) > 0 { - user.FullName = name - } +func (*DefaultUserInitStrategy) InitializeUser(identity *userapi.Identity, user *userapi.User) error { + if len(user.FullName) == 0 { + user.FullName = identity.Extra[authapi.IdentityDisplayNameKey] } return nil } diff --git a/pkg/user/initializer_test.go b/pkg/user/initializer_test.go new file mode 100644 index 000000000000..c19511cd9e02 --- /dev/null +++ b/pkg/user/initializer_test.go @@ -0,0 +1,52 @@ +package user + +import ( + "reflect" + "testing" + + "github.com/openshift/origin/pkg/user/api" +) + +func TestInitializerUser(t *testing.T) { + testcases := map[string]struct { + Identity *api.Identity + User *api.User + ExpectedUser *api.User + }{ + "empty": { + Identity: &api.Identity{}, + User: &api.User{}, + ExpectedUser: &api.User{}, + }, + "empty extra": { + Identity: &api.Identity{Extra: map[string]string{}}, + User: &api.User{}, + ExpectedUser: &api.User{}, + }, + "sets full name": { + Identity: &api.Identity{ + Extra: map[string]string{"name": "Bob"}, + }, + User: &api.User{}, + ExpectedUser: &api.User{FullName: "Bob"}, + }, + "respects existing full name": { + Identity: &api.Identity{ + Extra: map[string]string{"name": "Bob"}, + }, + User: &api.User{FullName: "Harold"}, + ExpectedUser: &api.User{FullName: "Harold"}, + }, + } + + for k, tc := range testcases { + err := NewDefaultUserInitStrategy().InitializeUser(tc.Identity, tc.User) + if err != nil { + t.Errorf("%s: unexpected error: %v", k, err) + continue + } + if !reflect.DeepEqual(tc.User, tc.ExpectedUser) { + t.Errorf("%s: expected \n\t%#v\ngot\n\t%#v", k, tc.ExpectedUser, tc.User) + } + } +} diff --git a/pkg/user/interfaces.go b/pkg/user/interfaces.go index b50748265edf..2341c104dcd9 100644 --- a/pkg/user/interfaces.go +++ b/pkg/user/interfaces.go @@ -4,6 +4,8 @@ import ( "github.com/openshift/origin/pkg/user/api" ) +// Initializer is responsible for initializing fields in a User API object from its associated Identity type Initializer interface { + // InitializeUser is responsible for initializing fields in a User API object from its associated Identity InitializeUser(identity *api.Identity, user *api.User) error } diff --git a/test/integration/auth_proxy_test.go b/test/integration/auth_proxy_test.go index dce0df99403f..872e68f61975 100644 --- a/test/integration/auth_proxy_test.go +++ b/test/integration/auth_proxy_test.go @@ -71,7 +71,10 @@ func TestAuthProxyOnAuthorize(t *testing.T) { identityStorage := identityetcd.NewREST(etcdHelper) identityRegistry := identityregistry.NewRegistry(identityStorage) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) + identityMapper, err := identitymapper.NewIdentityUserMapper(identityRegistry, userRegistry, identitymapper.MappingMethodGenerate) + if err != nil { + t.Fatal(err) + } // this auth request handler is the one that is supposed to recognize information from a front proxy authRequestHandler := headerrequest.NewAuthenticator("front-proxy-test", headerrequest.NewDefaultConfig(), identityMapper) diff --git a/test/integration/cli_get_token_test.go b/test/integration/cli_get_token_test.go index 94e3e5e1b81d..0d42918bd416 100644 --- a/test/integration/cli_get_token_test.go +++ b/test/integration/cli_get_token_test.go @@ -69,7 +69,10 @@ func TestCLIGetToken(t *testing.T) { identityStorage := identityetcd.NewREST(etcdHelper) identityRegistry := identityregistry.NewRegistry(identityStorage) - identityMapper := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) + identityMapper, err := identitymapper.NewIdentityUserMapper(identityRegistry, userRegistry, identitymapper.MappingMethodGenerate) + if err != nil { + t.Fatal(err) + } authRequestHandler := basicauthrequest.NewBasicAuthAuthentication(allowanypassword.New("get-token-test", identityMapper), true) authHandler := oauthhandlers.NewUnionAuthenticationHandler( diff --git a/test/integration/oauth_basicauth_test.go b/test/integration/oauth_basicauth_test.go index 79c1cbb47988..ef4c8d83f4fb 100644 --- a/test/integration/oauth_basicauth_test.go +++ b/test/integration/oauth_basicauth_test.go @@ -237,6 +237,7 @@ func TestOAuthBasicAuthPassword(t *testing.T) { Name: "basicauth", UseAsChallenger: true, UseAsLogin: true, + MappingMethod: "claim", Provider: runtime.EmbeddedObject{ Object: &configapi.BasicAuthPasswordIdentityProvider{ RemoteConnectionInfo: configapi.RemoteConnectionInfo{ diff --git a/test/integration/oauth_htpasswd_test.go b/test/integration/oauth_htpasswd_test.go index 3f7c26f95a58..d7f9f79fa150 100644 --- a/test/integration/oauth_htpasswd_test.go +++ b/test/integration/oauth_htpasswd_test.go @@ -17,7 +17,7 @@ import ( testserver "github.com/openshift/origin/test/util/server" ) -func TestHTPasswd(t *testing.T) { +func TestOAuthHTPasswd(t *testing.T) { htpasswdFile, err := ioutil.TempFile("", "test.htpasswd") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -33,6 +33,7 @@ func TestHTPasswd(t *testing.T) { Name: "htpasswd", UseAsChallenger: true, UseAsLogin: true, + MappingMethod: "claim", Provider: runtime.EmbeddedObject{ Object: &configapi.HTPasswdPasswordIdentityProvider{ File: htpasswdFile.Name(), diff --git a/test/integration/oauth_ldap_test.go b/test/integration/oauth_ldap_test.go index 08f965c6bbb3..858877e49780 100644 --- a/test/integration/oauth_ldap_test.go +++ b/test/integration/oauth_ldap_test.go @@ -86,6 +86,7 @@ func TestOAuthLDAP(t *testing.T) { Name: providerName, UseAsChallenger: true, UseAsLogin: true, + MappingMethod: "claim", Provider: runtime.EmbeddedObject{ Object: &configapi.LDAPPasswordIdentityProvider{ URL: fmt.Sprintf("ldap://%s/%s?%s?%s?%s", ldapAddress, searchDN, searchAttr, searchScope, searchFilter), diff --git a/test/integration/oauth_request_header_test.go b/test/integration/oauth_request_header_test.go index 89705947e5f2..aee841eb9d67 100644 --- a/test/integration/oauth_request_header_test.go +++ b/test/integration/oauth_request_header_test.go @@ -167,6 +167,7 @@ func TestOAuthRequestHeader(t *testing.T) { Name: "requestheader", UseAsChallenger: true, UseAsLogin: true, + MappingMethod: "claim", Provider: runtime.EmbeddedObject{ Object: &configapi.RequestHeaderIdentityProvider{ ChallengeURL: proxyServer.URL + "/oauth/authorize?${query}", diff --git a/test/integration/userclient_test.go b/test/integration/userclient_test.go index 858f0cde128b..a33982c9c0d6 100644 --- a/test/integration/userclient_test.go +++ b/test/integration/userclient_test.go @@ -4,6 +4,7 @@ package integration import ( "path" + "reflect" "sync" "testing" @@ -21,7 +22,6 @@ import ( 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" testserver "github.com/openshift/origin/test/util/server" ) @@ -83,20 +83,33 @@ func TestUserInitialization(t *testing.T) { etcdClient, err := etcd.GetAndTestEtcdClient(masterConfig.EtcdClientInfo) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } etcdHelper, err := origin.NewEtcdStorage(etcdClient, masterConfig.EtcdStorageConfig.OpenShiftStorageVersion, masterConfig.EtcdStorageConfig.OpenShiftStoragePrefix) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } userRegistry := userregistry.NewRegistry(useretcd.NewREST(etcdHelper)) identityRegistry := identityregistry.NewRegistry(identityetcd.NewREST(etcdHelper)) - useridentityMappingRegistry := useridentitymapping.NewRegistry(useridentitymapping.NewREST(userRegistry, identityRegistry)) - lookup := identitymapper.NewLookupIdentityMapper(useridentityMappingRegistry, userRegistry) - provisioner := identitymapper.NewAlwaysCreateUserIdentityToUserMapper(identityRegistry, userRegistry) + lookup, err := identitymapper.NewIdentityUserMapper(identityRegistry, userRegistry, identitymapper.MappingMethodLookup) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + generate, err := identitymapper.NewIdentityUserMapper(identityRegistry, userRegistry, identitymapper.MappingMethodGenerate) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + add, err := identitymapper.NewIdentityUserMapper(identityRegistry, userRegistry, identitymapper.MappingMethodAdd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + claim, err := identitymapper.NewIdentityUserMapper(identityRegistry, userRegistry, identitymapper.MappingMethodClaim) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } testcases := map[string]struct { Identity authapi.UserIdentityInfo @@ -107,9 +120,10 @@ func TestUserInitialization(t *testing.T) { CreateMapping *api.UserIdentityMapping UpdateUser *api.User - ExpectedErr error - ExpectedUserName string - ExpectedFullName string + ExpectedErr error + ExpectedUserName string + ExpectedFullName string + ExpectedIdentities []string }{ "lookup missing identity": { Identity: makeIdentityInfo("idp", "bob", nil), @@ -125,65 +139,245 @@ func TestUserInitialization(t *testing.T) { CreateIdentity: makeIdentity("idp", "bob"), CreateMapping: makeMapping("mappeduser", "idp:bob"), - ExpectedUserName: "mappeduser", + ExpectedUserName: "mappeduser", + ExpectedIdentities: []string{"idp:bob"}, }, - "provision missing identity and user": { + "generate missing identity and user": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: generate, - ExpectedUserName: "bob", + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, }, - "provision missing identity and user with preferred username and display name": { + "generate missing identity and user with preferred username and display name": { Identity: makeIdentityInfo("idp", "bob", map[string]string{authapi.IdentityDisplayNameKey: "Bob, Sr.", authapi.IdentityPreferredUsernameKey: "admin"}), - Mapper: provisioner, + Mapper: generate, - ExpectedUserName: "admin", - ExpectedFullName: "Bob, Sr.", + ExpectedUserName: "admin", + ExpectedFullName: "Bob, Sr.", + ExpectedIdentities: []string{"idp:bob"}, }, - "provision missing identity for existing user": { + "generate missing identity for existing user": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: generate, CreateUser: makeUser("bob", "idp:bob"), - ExpectedUserName: "bob", + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, }, - "provision missing identity with conflicting user": { + "generate missing identity with conflicting user": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: generate, CreateUser: makeUser("bob"), - ExpectedUserName: "bob2", + ExpectedUserName: "bob2", + ExpectedIdentities: []string{"idp:bob"}, }, - "provision missing identity with conflicting user and preferred username": { + "generate missing identity with conflicting user and preferred username": { Identity: makeIdentityInfo("idp", "bob", map[string]string{authapi.IdentityPreferredUsernameKey: "admin"}), - Mapper: provisioner, + Mapper: generate, CreateUser: makeUser("admin"), - ExpectedUserName: "admin2", + ExpectedUserName: "admin2", + ExpectedIdentities: []string{"idp:bob"}, + }, + "generate with existing unmapped identity": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: generate, + + CreateIdentity: makeIdentity("idp", "bob"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "generate with existing mapped identity with invalid user UID": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: generate, + + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentityWithUserReference("idp", "bob", "mappeduser", "invalidUID"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + ExpectedIdentities: []string{"idp:bob"}, + }, + "generate with existing mapped identity without user backreference": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: generate, + + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentity("idp", "bob"), + CreateMapping: makeMapping("mappeduser", "idp:bob"), + // Update user to a version which does not reference the identity + UpdateUser: makeUser("mappeduser"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "generate returns existing mapping": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: generate, + + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentity("idp", "bob"), + CreateMapping: makeMapping("mappeduser", "idp:bob"), + + ExpectedUserName: "mappeduser", + ExpectedIdentities: []string{"idp:bob"}, + }, + + "add missing identity and user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, + }, + "add missing identity and user with preferred username and display name": { + Identity: makeIdentityInfo("idp", "bob", map[string]string{authapi.IdentityDisplayNameKey: "Bob, Sr.", authapi.IdentityPreferredUsernameKey: "admin"}), + Mapper: add, + + ExpectedUserName: "admin", + ExpectedFullName: "Bob, Sr.", + ExpectedIdentities: []string{"idp:bob"}, + }, + "add missing identity for existing user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + CreateUser: makeUser("bob", "idp:bob"), + + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, + }, + "add missing identity with conflicting user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + CreateUser: makeUser("bob", "otheridp:otheruser"), + + ExpectedUserName: "bob", + ExpectedIdentities: []string{"otheridp:otheruser", "idp:bob"}, + }, + "add missing identity with conflicting user and preferred username": { + Identity: makeIdentityInfo("idp", "bob", map[string]string{authapi.IdentityPreferredUsernameKey: "admin"}), + Mapper: add, + + CreateUser: makeUser("admin", "otheridp:otheruser"), + + ExpectedUserName: "admin", + ExpectedIdentities: []string{"otheridp:otheruser", "idp:bob"}, + }, + "add with existing unmapped identity": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + CreateIdentity: makeIdentity("idp", "bob"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "add with existing mapped identity with invalid user UID": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentityWithUserReference("idp", "bob", "mappeduser", "invalidUID"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "add with existing mapped identity without user backreference": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentity("idp", "bob"), + CreateMapping: makeMapping("mappeduser", "idp:bob"), + // Update user to a version which does not reference the identity + UpdateUser: makeUser("mappeduser"), + + ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), + }, + "add returns existing mapping": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: add, + + CreateUser: makeUser("mappeduser"), + CreateIdentity: makeIdentity("idp", "bob"), + CreateMapping: makeMapping("mappeduser", "idp:bob"), + + ExpectedUserName: "mappeduser", + ExpectedIdentities: []string{"idp:bob"}, + }, + + "claim missing identity and user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: claim, + + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, + }, + "claim missing identity and user with preferred username and display name": { + Identity: makeIdentityInfo("idp", "bob", map[string]string{authapi.IdentityDisplayNameKey: "Bob, Sr.", authapi.IdentityPreferredUsernameKey: "admin"}), + Mapper: claim, + + ExpectedUserName: "admin", + ExpectedFullName: "Bob, Sr.", + ExpectedIdentities: []string{"idp:bob"}, + }, + "claim missing identity for existing user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: claim, + + CreateUser: makeUser("bob", "idp:bob"), + + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, }, - "provision with existing unmapped identity": { + "claim missing identity with existing available user": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: claim, + + CreateUser: makeUser("bob"), + + ExpectedUserName: "bob", + ExpectedIdentities: []string{"idp:bob"}, + }, + "claim missing identity with conflicting user": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: claim, + + CreateUser: makeUser("bob", "otheridp:otheruser"), + + ExpectedErr: identitymapper.NewClaimError(makeUser("bob", "otheridp:otheruser"), makeIdentity("idp", "bob")), + }, + "claim missing identity with conflicting user and preferred username": { + Identity: makeIdentityInfo("idp", "bob", map[string]string{authapi.IdentityPreferredUsernameKey: "admin"}), + Mapper: claim, + + CreateUser: makeUser("admin", "otheridp:otheruser"), + + ExpectedErr: identitymapper.NewClaimError(makeUser("admin", "otheridp:otheruser"), makeIdentity("idp", "bob")), + }, + "claim with existing unmapped identity": { + Identity: makeIdentityInfo("idp", "bob", nil), + Mapper: claim, CreateIdentity: makeIdentity("idp", "bob"), ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), }, - "provision with existing mapped identity with invalid user UID": { + "claim with existing mapped identity with invalid user UID": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: claim, CreateUser: makeUser("mappeduser"), CreateIdentity: makeIdentityWithUserReference("idp", "bob", "mappeduser", "invalidUID"), ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), }, - "provision with existing mapped identity without user backreference": { + "claim with existing mapped identity without user backreference": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: claim, CreateUser: makeUser("mappeduser"), CreateIdentity: makeIdentity("idp", "bob"), @@ -193,15 +387,16 @@ func TestUserInitialization(t *testing.T) { ExpectedErr: kerrs.NewNotFound("UserIdentityMapping", "idp:bob"), }, - "provision returns existing mapping": { + "claim returns existing mapping": { Identity: makeIdentityInfo("idp", "bob", nil), - Mapper: provisioner, + Mapper: claim, CreateUser: makeUser("mappeduser"), CreateIdentity: makeIdentity("idp", "bob"), CreateMapping: makeMapping("mappeduser", "idp:bob"), - ExpectedUserName: "mappeduser", + ExpectedUserName: "mappeduser", + ExpectedIdentities: []string{"idp:bob"}, }, } @@ -285,7 +480,9 @@ func TestUserInitialization(t *testing.T) { if user.FullName != testcase.ExpectedFullName { t.Errorf("%s: Expected full name %s, got %s", k, testcase.ExpectedFullName, user.FullName) } - + if !reflect.DeepEqual(user.Identities, testcase.ExpectedIdentities) { + t.Errorf("%s: Expected identities %v, got %v", k, testcase.ExpectedIdentities, user.Identities) + } }() } wg.Wait()