From 03ffd0798c5c4c33190731435bb84ed085585da4 Mon Sep 17 00:00:00 2001
From: James Nord <jnord+github@cloudbees.com>
Date: Thu, 31 Jan 2019 18:21:54 +0000
Subject: [PATCH] Allow an option to use the github user handle rather than an
 id.

For downstream apps using a github handle is much simpler than working
with numbers.

WHilst the number is stable and the handle is not - GitHUb does give you
a big scary wanring if you try and change it that bad things may happen
to you, and generally few users ever change it.

This can be enabled with a configuration option `useLoginAsId`
---
 connector/github/github.go      |  9 ++++++++
 connector/github/github_test.go | 38 ++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/connector/github/github.go b/connector/github/github.go
index 07db4b3b..9d900414 100644
--- a/connector/github/github.go
+++ b/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,17 @@ 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) {
diff --git a/connector/github/github_test.go b/connector/github/github_test.go
index 38a9a6cf..698bb4ce 100644
--- a/connector/github/github_test.go
+++ b/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,42 @@ 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 {