Merge pull request #1396 from jtnord/useLoginId-dexidp

Use github login as the id

Documentation/connectors/github.md

@@ -67,6 +67,9 @@ connectors:
     #   - ['acme:site-reliability-engineers'] for 'slug'
     #   - ['acme:Site Reliability Engineers', 'acme:site-reliability-engineers'] for 'both'
     teamNameField: slug
+    # flag which will switch from using the internal GitHub id to the users handle (@mention) as the user id.
+    # It is possible for a user to change their own user name but it is very rare for them to do so
+    useLoginAsID: false
 ```
 
 ## GitHub Enterprise

connector/github/github.go

@@ -48,6 +48,7 @@ type Config struct {
 	RootCA        string `json:"rootCA"`
 	TeamNameField string `json:"teamNameField"`
 	LoadAllGroups bool   `json:"loadAllGroups"`
+	UseLoginAsID  bool   `json:"useLoginAsID"`
 }
 
 // Org holds org-team filters, in which teams are optional.
@@ -83,6 +84,7 @@ func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector
 		clientSecret: c.ClientSecret,
 		apiURL:       apiURL,
 		logger:       logger,
+		useLoginAsID: c.UseLoginAsID,
 	}
 
 	if c.HostName != "" {
@@ -148,6 +150,8 @@ type githubConnector struct {
 	teamNameField string
 	// if set to true and no orgs are configured then connector loads all user claims (all orgs and team)
 	loadAllGroups bool
+	// if set to true will use the users handle rather than their numeric id as the ID
+	useLoginAsID bool
 }
 
 // groupsRequired returns whether dex requires GitHub's 'read:org' scope. Dex
@@ -260,12 +264,16 @@ func (c *githubConnector) HandleCallback(s connector.Scopes, r *http.Request) (i
 	if username == "" {
 		username = user.Login
 	}
+
 	identity = connector.Identity{
 		UserID:        strconv.Itoa(user.ID),
 		Username:      username,
 		Email:         user.Email,
 		EmailVerified: true,
 	}
+	if c.useLoginAsID {
+		identity.UserID = user.Login
+	}
 
 	// Only set identity.Groups if 'orgs', 'org', or 'groups' scope are specified.
 	if c.groupsRequired(s.Groups) {

connector/github/github_test.go

@@ -124,10 +124,11 @@ func TestUserGroupsWithTeamNameAndSlugFieldConfig(t *testing.T) {
 	})
 }
 
+// tests that the users login is used as their username when they have no username set
 func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
 
 	s := newTestServer(map[string]testResponse{
-		"/user": {data: user{Login: "some-login"}},
+		"/user": {data: user{Login: "some-login", ID: 12345678}},
 		"/user/emails": {data: []userEmail{{
 			Email:    "some@email.com",
 			Verified: true,
@@ -154,6 +155,7 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
 
 	expectNil(t, err)
 	expectEquals(t, identity.Username, "some-login")
+	expectEquals(t, identity.UserID, "12345678")
 	expectEquals(t, 0, len(identity.Groups))
 
 	c = githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient(), loadAllGroups: true}
@@ -161,8 +163,41 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
 
 	expectNil(t, err)
 	expectEquals(t, identity.Username, "some-login")
+	expectEquals(t, identity.UserID, "12345678")
 	expectEquals(t, identity.Groups, []string{"org-1"})
+}
 
+func TestLoginUsedAsIDWhenConfigured(t *testing.T) {
+
+	s := newTestServer(map[string]testResponse{
+		"/user": {data: user{Login: "some-login", ID: 12345678, Name: "Joe Bloggs"}},
+		"/user/emails": {data: []userEmail{{
+			Email:    "some@email.com",
+			Verified: true,
+			Primary:  true,
+		}}},
+		"/login/oauth/access_token": {data: map[string]interface{}{
+			"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
+			"expires_in":   "30",
+		}},
+		"/user/orgs": {
+			data: []org{{Login: "org-1"}},
+		},
+	})
+	defer s.Close()
+
+	hostURL, err := url.Parse(s.URL)
+	expectNil(t, err)
+
+	req, err := http.NewRequest("GET", hostURL.String(), nil)
+	expectNil(t, err)
+
+	c := githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient(), useLoginAsID: true}
+	identity, err := c.HandleCallback(connector.Scopes{Groups: true}, req)
+
+	expectNil(t, err)
+	expectEquals(t, identity.UserID, "some-login")
+	expectEquals(t, identity.Username, "Joe Bloggs")
 }
 
 func newTestServer(responses map[string]testResponse) *httptest.Server {