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`
This commit is contained in:
James Nord 2019-01-31 18:21:54 +00:00
parent b5826e66f0
commit 03ffd0798c
2 changed files with 46 additions and 1 deletions

View file

@ -48,6 +48,7 @@ type Config struct {
RootCA string `json:"rootCA"` RootCA string `json:"rootCA"`
TeamNameField string `json:"teamNameField"` TeamNameField string `json:"teamNameField"`
LoadAllGroups bool `json:"loadAllGroups"` LoadAllGroups bool `json:"loadAllGroups"`
UseLoginAsId bool `json:"useLoginAsId"`
} }
// Org holds org-team filters, in which teams are optional. // 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, clientSecret: c.ClientSecret,
apiURL: apiURL, apiURL: apiURL,
logger: logger, logger: logger,
useLoginAsId: c.UseLoginAsId,
} }
if c.HostName != "" { if c.HostName != "" {
@ -148,6 +150,8 @@ type githubConnector struct {
teamNameField string teamNameField string
// if set to true and no orgs are configured then connector loads all user claims (all orgs and team) // if set to true and no orgs are configured then connector loads all user claims (all orgs and team)
loadAllGroups bool 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 // 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 == "" { if username == "" {
username = user.Login username = user.Login
} }
identity = connector.Identity{ identity = connector.Identity{
UserID: strconv.Itoa(user.ID), UserID: strconv.Itoa(user.ID),
Username: username, Username: username,
Email: user.Email, Email: user.Email,
EmailVerified: true, EmailVerified: true,
} }
if c.useLoginAsId {
identity.UserID = user.Login
}
// Only set identity.Groups if 'orgs', 'org', or 'groups' scope are specified. // Only set identity.Groups if 'orgs', 'org', or 'groups' scope are specified.
if c.groupsRequired(s.Groups) { if c.groupsRequired(s.Groups) {

View file

@ -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) { func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
s := newTestServer(map[string]testResponse{ s := newTestServer(map[string]testResponse{
"/user": {data: user{Login: "some-login"}}, "/user": {data: user{Login: "some-login", ID: 12345678}},
"/user/emails": {data: []userEmail{{ "/user/emails": {data: []userEmail{{
Email: "some@email.com", Email: "some@email.com",
Verified: true, Verified: true,
@ -154,6 +155,7 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
expectNil(t, err) expectNil(t, err)
expectEquals(t, identity.Username, "some-login") expectEquals(t, identity.Username, "some-login")
expectEquals(t, identity.UserID, "12345678")
expectEquals(t, 0, len(identity.Groups)) expectEquals(t, 0, len(identity.Groups))
c = githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient(), loadAllGroups: true} c = githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient(), loadAllGroups: true}
@ -161,8 +163,42 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
expectNil(t, err) expectNil(t, err)
expectEquals(t, identity.Username, "some-login") expectEquals(t, identity.Username, "some-login")
expectEquals(t, identity.UserID, "12345678")
expectEquals(t, identity.Groups, []string{"org-1"}) 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 { func newTestServer(responses map[string]testResponse) *httptest.Server {