Skip to content

Commit

Permalink
Merge pull request dexidp#1349 from alexmt/1102-config-to-load-all-gr…
Browse files Browse the repository at this point in the history
…oups

Add config to explicitly enable loading all github groups

Follow-up for dexidp#1102.
  • Loading branch information
srenatus authored Nov 20, 2018
2 parents cbd9092 + 41b1360 commit 175893c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
6 changes: 4 additions & 2 deletions Documentation/connectors/github.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ connectors:
# If orgs are specified in the config then user MUST be a member of at least one of the specified orgs to
# authenticate with dex.
#
# If neither 'org' nor 'orgs' are specified in the config then user authenticate with ALL user's Github groups.
# Typical use case for this setup:
# If neither 'org' nor 'orgs' are specified in the config and 'loadAllGroups' setting set to true then user
# authenticate with ALL user's Github groups. Typical use case for this setup:
# provide read-only access to everyone and give full permissions if user has 'my-organization:admins-team' group claim.
orgs:
- name: my-organization
Expand All @@ -56,6 +56,8 @@ connectors:
teams:
- red-team
- blue-team
# Flag which indicates that all user groups and teams should be loaded.
loadAllGroups: false

# Optional choice between 'name' (default) or 'slug'.
#
Expand Down
9 changes: 7 additions & 2 deletions connector/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type Config struct {
HostName string `json:"hostName"`
RootCA string `json:"rootCA"`
TeamNameField string `json:"teamNameField"`
LoadAllGroups bool `json:"loadAllGroups"`
}

// Org holds org-team filters, in which teams are optional.
Expand Down Expand Up @@ -107,6 +108,7 @@ func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector
}

}
g.loadAllGroups = c.LoadAllGroups

switch c.TeamNameField {
case "name", "slug", "":
Expand Down Expand Up @@ -142,8 +144,11 @@ type githubConnector struct {
// Used to support untrusted/self-signed CA certs.
rootCA string
// HTTP Client that trusts the custom delcared rootCA cert.
httpClient *http.Client
httpClient *http.Client
// optional choice between 'name' (default) or 'slug'
teamNameField string
// if set to true and no orgs are configured then connector loads all user claims (all orgs and team)
loadAllGroups bool
}

// groupsRequired returns whether dex requires GitHub's 'read:org' scope. Dex
Expand Down Expand Up @@ -325,7 +330,7 @@ func (c *githubConnector) getGroups(ctx context.Context, client *http.Client, gr
return c.groupsForOrgs(ctx, client, userLogin)
} else if c.org != "" {
return c.teamsForOrg(ctx, client, c.org)
} else if groupScope {
} else if groupScope && c.loadAllGroups {
return c.userGroups(ctx, client)
}
return nil, nil
Expand Down
13 changes: 12 additions & 1 deletion connector/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
"expires_in": "30",
}},
"/user/orgs": {
data: []org{{Login: "org-1"}},
},
})
defer s.Close()

Expand All @@ -125,10 +128,18 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
expectNil(t, err)

c := githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient()}
identity, err := c.HandleCallback(connector.Scopes{}, req)
identity, err := c.HandleCallback(connector.Scopes{Groups: true}, req)

expectNil(t, err)
expectEquals(t, identity.Username, "some-login")
expectEquals(t, 0, len(identity.Groups))

c = githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient(), loadAllGroups: true}
identity, err = c.HandleCallback(connector.Scopes{Groups: true}, req)

expectNil(t, err)
expectEquals(t, identity.Username, "some-login")
expectEquals(t, identity.Groups, []string{"org-1"})

}

Expand Down

0 comments on commit 175893c

Please sign in to comment.