diff --git a/Documentation/connectors/github.md b/Documentation/connectors/github.md index 78f7def4..f9395283 100644 --- a/Documentation/connectors/github.md +++ b/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 diff --git a/connector/github/github.go b/connector/github/github.go index 07db4b3b..9f7c2782 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,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) { diff --git a/connector/github/github_test.go b/connector/github/github_test.go index 38a9a6cf..539a2e69 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,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 {