From 7bd084bc0761a5fe612fe4e06ce385b1b25e879b Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Mon, 19 Nov 2018 10:14:38 -0800 Subject: [PATCH] Issue #1102 - Add config to explicitly enable loading all github groups --- Documentation/connectors/github.md | 6 ++++-- connector/github/github.go | 9 +++++++-- connector/github/github_test.go | 13 ++++++++++++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Documentation/connectors/github.md b/Documentation/connectors/github.md index ddf11869..fa8f0eb2 100644 --- a/Documentation/connectors/github.md +++ b/Documentation/connectors/github.md @@ -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 @@ -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'. # diff --git a/connector/github/github.go b/connector/github/github.go index 977d190f..48efd52d 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -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. @@ -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", "": @@ -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 @@ -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 diff --git a/connector/github/github_test.go b/connector/github/github_test.go index 7069091d..9220cc62 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -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() @@ -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"}) }