Browse code

Change Identity.Name to Identity.UserName

Jordan Liggitt authored on 2014/11/21 06:12:34
Showing 13 changed files
... ...
@@ -11,8 +11,8 @@ type UserInfo interface {
11 11
 // UserIdentityInfo contains information about an identity.  Identities are distinct from users.  An authentication server of
12 12
 // some kind (like oauth for example) describes an identity.  Our system controls the users mapped to this identity.
13 13
 type UserIdentityInfo interface {
14
-	// GetName uniquely identifies this particular identity for this provider.  It is NOT guaranteed to be unique across providers
15
-	GetName() string
14
+	// GetUserName uniquely identifies this particular identity for this provider.  It is NOT guaranteed to be unique across providers
15
+	GetUserName() string
16 16
 	// GetProviderName returns the name of the provider of this identity.
17 17
 	GetProviderName() string
18 18
 	// GetExtra is a map to allow providers to add additional fields that they understand
... ...
@@ -64,21 +64,21 @@ func (i *DefaultUserInfo) GetExtra() map[string]string {
64 64
 }
65 65
 
66 66
 type DefaultUserIdentityInfo struct {
67
-	Name         string
67
+	UserName     string
68 68
 	ProviderName string
69 69
 	Extra        map[string]string
70 70
 }
71 71
 
72 72
 // NewDefaultUserIdentityInfo returns a DefaultUserIdentity info with a non-nil Extra component
73
-func NewDefaultUserIdentityInfo(name string) DefaultUserIdentityInfo {
73
+func NewDefaultUserIdentityInfo(username string) DefaultUserIdentityInfo {
74 74
 	return DefaultUserIdentityInfo{
75
-		Name:  name,
76
-		Extra: make(map[string]string),
75
+		UserName: username,
76
+		Extra:    make(map[string]string),
77 77
 	}
78 78
 }
79 79
 
80
-func (i *DefaultUserIdentityInfo) GetName() string {
81
-	return i.Name
80
+func (i *DefaultUserIdentityInfo) GetUserName() string {
81
+	return i.UserName
82 82
 }
83 83
 
84 84
 func (i *DefaultUserIdentityInfo) GetProviderName() string {
... ...
@@ -26,7 +26,7 @@ func (a alwaysAcceptPasswordAuthenticator) AuthenticatePassword(username, passwo
26 26
 	}
27 27
 
28 28
 	identity := &authapi.DefaultUserIdentityInfo{
29
-		Name: username,
29
+		UserName: username,
30 30
 	}
31 31
 	user, err := a.identityMapper.UserFor(identity)
32 32
 	glog.V(4).Infof("Got userIdentityMapping: %#v", user)
... ...
@@ -90,7 +90,7 @@ func (a *Authenticator) AuthenticatePassword(username, password string) (api.Use
90 90
 	}
91 91
 
92 92
 	identity := &authapi.DefaultUserIdentityInfo{
93
-		Name: username,
93
+		UserName: username,
94 94
 		Extra: map[string]string{
95 95
 			"name":  remoteUserData.Name,
96 96
 			"email": remoteUserData.Email,
... ...
@@ -80,7 +80,7 @@ func (p provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentit
80 80
 	}
81 81
 
82 82
 	identity := &authapi.DefaultUserIdentityInfo{
83
-		Name: fmt.Sprintf("%d", userdata.ID),
83
+		UserName: fmt.Sprintf("%d", userdata.ID),
84 84
 		Extra: map[string]string{
85 85
 			"name":  userdata.Name,
86 86
 			"login": userdata.Login,
... ...
@@ -67,7 +67,7 @@ func (p provider) GetUserIdentity(data *osincli.AccessData) (authapi.UserIdentit
67 67
 	}
68 68
 
69 69
 	identity := &authapi.DefaultUserIdentityInfo{
70
-		Name: id,
70
+		UserName: id,
71 71
 		Extra: map[string]string{
72 72
 			"name":  email,
73 73
 			"email": email,
... ...
@@ -1,7 +1,8 @@
1 1
 package identitymapper
2 2
 
3 3
 import (
4
-	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
4
+	"errors"
5
+
5 6
 	authapi "github.com/openshift/origin/pkg/auth/api"
6 7
 	userapi "github.com/openshift/origin/pkg/user/api"
7 8
 	"github.com/openshift/origin/pkg/user/registry/useridentitymapping"
... ...
@@ -21,14 +22,18 @@ func NewAlwaysCreateUserIdentityToUserMapper(providerId string, userIdentityRegi
21 21
 func (p *alwaysCreateUserIdentityToUserMapper) UserFor(identityInfo authapi.UserIdentityInfo) (authapi.UserInfo, error) {
22 22
 	userIdentityMapping := &userapi.UserIdentityMapping{
23 23
 		Identity: userapi.Identity{
24
-			ObjectMeta: kapi.ObjectMeta{
25
-				Name: identityInfo.GetName(),
26
-			},
27
-			Provider: p.providerId, // Provider information from the provider plugin itself is not considered authoritative
24
+			Provider: p.providerId, // Provider id is imposed
25
+			UserName: identityInfo.GetUserName(),
28 26
 			Extra:    identityInfo.GetExtra(),
29 27
 		},
30 28
 	}
31
-	authoritativeMapping, _, err := p.userIdentityRegistry.CreateOrUpdateUserIdentityMapping(userIdentityMapping)
29
+	authoritativeMapping, ok, err := p.userIdentityRegistry.CreateOrUpdateUserIdentityMapping(userIdentityMapping)
30
+	if err != nil {
31
+		return nil, err
32
+	}
33
+	if !ok {
34
+		return nil, errors.New("Could not map identity to user")
35
+	}
32 36
 
33 37
 	ret := &authapi.DefaultUserInfo{
34 38
 		Name:  authoritativeMapping.User.Name,
... ...
@@ -12,15 +12,15 @@ func TestProvisionUser(t *testing.T) {
12 12
 	providerId := "papa"
13 13
 	identityMapper := NewAlwaysCreateUserIdentityToUserMapper(providerId, userIdentityRegistry)
14 14
 	identity := &authapi.DefaultUserIdentityInfo{
15
-		Name: "oscar",
15
+		UserName: "oscar",
16 16
 	}
17 17
 
18 18
 	identityMapper.UserFor(identity)
19 19
 	if userIdentityRegistry.CreatedUserIdentityMapping.Identity.Provider != providerId {
20 20
 		t.Errorf("Expected provider to be set to %v, but it was actually %v", providerId, userIdentityRegistry.CreatedUserIdentityMapping.Identity.Provider)
21 21
 	}
22
-	if userIdentityRegistry.CreatedUserIdentityMapping.Identity.Name != identity.GetName() {
23
-		t.Errorf("Expected name to be set to %v, but it was actually %v", identity.GetName(), userIdentityRegistry.CreatedUserIdentityMapping.Identity.Name)
22
+	if userIdentityRegistry.CreatedUserIdentityMapping.Identity.UserName != identity.GetUserName() {
23
+		t.Errorf("Expected username to be set to %v, but it was actually %v", identity.GetUserName(), userIdentityRegistry.CreatedUserIdentityMapping.Identity.UserName)
24 24
 	}
25 25
 
26 26
 }
... ...
@@ -28,6 +28,9 @@ type Identity struct {
28 28
 	// is assumed.
29 29
 	Provider string `json:"provider" yaml:"provider"`
30 30
 
31
+	// UserName uniquely represents this identity in the scope of the identity provider
32
+	UserName string `json:"userName" yaml:"userName"`
33
+
31 34
 	Extra map[string]string `json:"extra,omitempty" yaml:"extra,omitempty"`
32 35
 }
33 36
 
... ...
@@ -28,6 +28,9 @@ type Identity struct {
28 28
 	// is assumed.
29 29
 	Provider string `json:"provider" yaml:"provider"`
30 30
 
31
+	// UserName uniquely represents this identity in the scope of the identity provider
32
+	UserName string `json:"userName" yaml:"userName"`
33
+
31 34
 	Extra map[string]string `json:"extra,omitempty" yaml:"extra,omitempty"`
32 35
 }
33 36
 
... ...
@@ -18,7 +18,7 @@ func (*DefaultUserInitStrategy) InitializeUser(identity *api.Identity, user *api
18 18
 	}
19 19
 	name, ok := identity.Extra["name"]
20 20
 	if !ok {
21
-		name = identity.Name
21
+		name = identity.UserName
22 22
 	}
23 23
 	user.FullName = name
24 24
 	return nil
... ...
@@ -41,7 +41,9 @@ func (r *Etcd) GetUser(name string) (user *api.User, err error) {
41 41
 
42 42
 // CreateOrUpdateUserIdentityMapping implements useridentitymapping.Registry
43 43
 func (r *Etcd) CreateOrUpdateUserIdentityMapping(mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, bool, error) {
44
-	name := fmt.Sprintf("%s:%s", mapping.Identity.Provider, mapping.Identity.Name)
44
+	// Create Identity.Name by combining Provider and UserName
45
+	name := fmt.Sprintf("%s:%s", mapping.Identity.Provider, mapping.Identity.UserName)
46
+	mapping.Identity.Name = name
45 47
 	key := makeUserKey(name)
46 48
 
47 49
 	// track the object we set into etcd to return
... ...
@@ -24,22 +24,23 @@ func NewTestEtcd(client tools.EtcdClient) *Etcd {
24 24
 // expect it to. If someone changes the location of these resources by say moving all the resources to
25 25
 // "/origin/resources" (which is a really good idea), then they've made a breaking change and something should
26 26
 // fail to let them know they've change some significant change and that other dependent pieces may break.
27
-func makeTestUserIdentityMapping(providerId, identityName string) string {
28
-	return fmt.Sprintf("/userIdentityMappings/%s:%s", providerId, identityName)
27
+func makeTestUserIdentityMapping(providerId, userName string) string {
28
+	return fmt.Sprintf("/userIdentityMappings/%s:%s", providerId, userName)
29 29
 }
30 30
 
31 31
 func TestEtcdGetUser(t *testing.T) {
32 32
 	fakeClient := tools.NewFakeEtcdClient(t)
33 33
 	expectedResultingMapping := &userapi.UserIdentityMapping{
34 34
 		Identity: userapi.Identity{
35
-			ObjectMeta: kapi.ObjectMeta{Name: "tango"},
35
+			ObjectMeta: kapi.ObjectMeta{Name: "victor:tango"},
36 36
 			Provider:   "victor",
37
+			UserName:   "tango",
37 38
 		},
38 39
 		User: userapi.User{
39 40
 			ObjectMeta: kapi.ObjectMeta{Name: "uniform"},
40 41
 		},
41 42
 	}
42
-	key := makeTestUserIdentityMapping(expectedResultingMapping.Identity.Provider, expectedResultingMapping.Identity.Name)
43
+	key := makeTestUserIdentityMapping(expectedResultingMapping.Identity.Provider, expectedResultingMapping.Identity.UserName)
43 44
 	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, expectedResultingMapping), 0)
44 45
 	registry := NewTestEtcd(fakeClient)
45 46
 
... ...
@@ -66,13 +67,14 @@ func TestEtcdCreateUserIdentityMapping(t *testing.T) {
66 66
 	expectedResultingMapping := &userapi.UserIdentityMapping{
67 67
 		ObjectMeta: kapi.ObjectMeta{Name: "quebec"},
68 68
 		Identity: userapi.Identity{
69
-			Provider: "sierra",
69
+			ObjectMeta: kapi.ObjectMeta{Name: "sierra:"},
70
+			Provider:   "sierra",
70 71
 		},
71 72
 		User: userapi.User{
72 73
 			ObjectMeta: kapi.ObjectMeta{Name: "romeo"},
73 74
 		},
74 75
 	}
75
-	key := makeTestUserIdentityMapping(testMapping.Identity.Provider, testMapping.Identity.Name)
76
+	key := makeTestUserIdentityMapping(testMapping.Identity.Provider, testMapping.Identity.UserName)
76 77
 
77 78
 	fakeClient.Data[key] = tools.EtcdResponseWithError{
78 79
 		R: &etcd.Response{
... ...
@@ -101,11 +103,12 @@ func TestEtcdUpdateUserIdentityMappingWithConflictingUser(t *testing.T) {
101 101
 	startingMapping := &userapi.UserIdentityMapping{
102 102
 		ObjectMeta: kapi.ObjectMeta{Name: "whiskey"},
103 103
 		Identity: userapi.Identity{
104
-			ObjectMeta: kapi.ObjectMeta{Name: "xray"},
104
+			ObjectMeta: kapi.ObjectMeta{Name: "yankee:xray"},
105 105
 			Provider:   "yankee",
106
+			UserName:   "xray",
106 107
 		},
107 108
 		User: userapi.User{
108
-			ObjectMeta: kapi.ObjectMeta{Name: "xray"},
109
+			ObjectMeta: kapi.ObjectMeta{Name: "yankee:xray"},
109 110
 		},
110 111
 	}
111 112
 	// this key is intentionally wrong so that we can have an internally consistend UserIdentityMapping
... ...
@@ -117,8 +120,8 @@ func TestEtcdUpdateUserIdentityMappingWithConflictingUser(t *testing.T) {
117 117
 	testMapping := &userapi.UserIdentityMapping{
118 118
 		ObjectMeta: kapi.ObjectMeta{Name: "bravo"},
119 119
 		Identity: userapi.Identity{
120
-			Provider:   "zulu",
121
-			ObjectMeta: kapi.ObjectMeta{Name: "alfa"},
120
+			Provider: "zulu",
121
+			UserName: "alfa",
122 122
 		},
123 123
 		User: userapi.User{},
124 124
 	}
... ...
@@ -147,7 +150,7 @@ func compareUserIdentityMappingFieldsThatAreFixed(expected, actual *userapi.User
147 147
 	if actual.Name != expected.Name {
148 148
 		return false
149 149
 	}
150
-	if actual.Identity.Name != expected.Identity.Name || actual.Identity.Provider != expected.Identity.Provider {
150
+	if actual.Identity.UserName != expected.Identity.UserName || actual.Identity.Provider != expected.Identity.Provider {
151 151
 		return false
152 152
 	}
153 153
 	if actual.User.Name != expected.User.Name || actual.User.Name != actual.User.Name {
... ...
@@ -47,8 +47,9 @@ func TestUserInitialization(t *testing.T) {
47 47
 
48 48
 	mapping := api.UserIdentityMapping{
49 49
 		Identity: api.Identity{
50
-			ObjectMeta: kapi.ObjectMeta{Name: "test"},
50
+			ObjectMeta: kapi.ObjectMeta{Name: ":test"},
51 51
 			Provider:   "",
52
+			UserName:   "test",
52 53
 			Extra: map[string]string{
53 54
 				"name": "Mr. Test",
54 55
 			},
... ...
@@ -133,8 +134,9 @@ func TestUserLookup(t *testing.T) {
133 133
 
134 134
 	mapping := api.UserIdentityMapping{
135 135
 		Identity: api.Identity{
136
-			ObjectMeta: kapi.ObjectMeta{Name: "test"},
136
+			ObjectMeta: kapi.ObjectMeta{Name: ":test"},
137 137
 			Provider:   "",
138
+			UserName:   "test",
138 139
 			Extra: map[string]string{
139 140
 				"name": "Mr. Test",
140 141
 			},