From 51d9b3d3ca654954b268d13ab65fa59e4014584a Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Wed, 14 Nov 2018 15:31:31 -0800 Subject: [PATCH 1/7] Issue #1184 - Github connector now returns a full group list when no org is specified --- connector/github/github.go | 95 +++++++++++++++++++++++-- connector/github/github_test.go | 121 ++++++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 7 deletions(-) create mode 100644 connector/github/github_test.go diff --git a/connector/github/github.go b/connector/github/github.go index 831a8f13..edb821ea 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -325,10 +325,17 @@ 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 { + return c.userGroups(ctx, client) } return nil, nil } +// formatTeamName return unique team name: prgs might have the same team names team name should be prefixed with org name to make team names unique across orgs. +func formatTeamName(org string, team string) string { + return fmt.Sprintf("%s:%s", org, team) +} + // groupsForOrgs enforces org and team constraints on user authorization // Cases in which user is authorized: // N orgs, no teams: user is member of at least 1 org @@ -359,11 +366,8 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client c.logger.Infof("github: user %q in org %q but no teams", userName, org.Name) } - // Orgs might have the same team names. We append orgPrefix to team name, - // i.e. "org:team", to make team names unique across orgs. - orgPrefix := org.Name + ":" for _, teamName := range teams { - groups = append(groups, orgPrefix+teamName) + groups = append(groups, formatTeamName(org.Name, teamName)) } } if inOrgNoTeams || len(groups) > 0 { @@ -372,6 +376,81 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client return groups, fmt.Errorf("github: user %q not in required orgs or teams", userName) } +func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) (groups []string, err error) { + orgs, err := c.userOrgs(ctx, client) + if err != nil { + return groups, err + } + + orgTeams, err := c.userOrgTeams(ctx, client) + if err != nil { + return groups, err + } + + for _, org := range orgs { + if teams, ok := orgTeams[org]; !ok { + groups = append(groups, org) + } else { + for _, team := range teams { + groups = append(groups, formatTeamName(org, team)) + } + } + } + + return groups, err +} + +// userOrgs retrieves list of current user orgs +func (c *githubConnector) userOrgs(ctx context.Context, client *http.Client) ([]string, error) { + apiURL, groups := c.apiURL+"/user/orgs", []string{} + for { + // https://developer.github.com/v3/orgs/#list-your-organizations + var ( + orgs []org + err error + ) + if apiURL, err = get(ctx, client, apiURL, &orgs); err != nil { + return nil, fmt.Errorf("github: get orgs: %v", err) + } + + for _, org := range orgs { + groups = append(groups, org.Login) + } + + if apiURL == "" { + break + } + } + + return groups, nil +} + +// userOrgTeams retrieves teams which current user belongs to. +// Method returns a map where key is an org name and value list of teams under the org. +func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) (map[string][]string, error) { + apiURL, groups := c.apiURL+"/user/teams", map[string][]string{} + for { + // https://developer.github.com/v3/orgs/teams/#list-user-teams + var ( + teams []team + err error + ) + if apiURL, err = get(ctx, client, apiURL, &teams); err != nil { + return nil, fmt.Errorf("github: get teams: %v", err) + } + + for _, team := range teams { + groups[team.Org.Login] = append(groups[team.Org.Login], team.Name) + } + + if apiURL == "" { + break + } + } + + return groups, nil +} + // Filter the users' team memberships by 'teams' from config. func filterTeams(userTeams, configTeams []string) (teams []string) { teamFilter := make(map[string]struct{}) @@ -572,12 +651,14 @@ func (c *githubConnector) userInOrg(ctx context.Context, client *http.Client, us // https://developer.github.com/v3/orgs/teams/#response-12 type team struct { Name string `json:"name"` - Org struct { - Login string `json:"login"` - } `json:"organization"` + Org org `json:"organization"` Slug string `json:"slug"` } +type org struct { + Login string `json:"login"` +} + // teamsForOrg queries the GitHub API for team membership within a specific organization. // // The HTTP passed client is expected to be constructed by the golang.org/x/oauth2 package, diff --git a/connector/github/github_test.go b/connector/github/github_test.go new file mode 100644 index 00000000..57aef554 --- /dev/null +++ b/connector/github/github_test.go @@ -0,0 +1,121 @@ +package github + +import ( + "context" + "crypto/tls" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "testing" + + "github.com/dexidp/dex/connector" +) + +func TestUserGroups(t *testing.T) { + + orgs := []org{ + {Login: "org-1"}, + {Login: "org-2"}, + {Login: "org-3"}, + } + + teams := []team{ + {Name: "team-1", Org: org{Login: "org-1"}}, + {Name: "team-2", Org: org{Login: "org-1"}}, + {Name: "team-3", Org: org{Login: "org-1"}}, + {Name: "team-4", Org: org{Login: "org-2"}}, + } + + s := newTestServer(map[string]interface{}{ + "/user/orgs": orgs, + "/user/teams": teams, + }) + + connector := githubConnector{apiURL: s.URL} + groups, err := connector.userGroups(context.Background(), newClient()) + + expectNil(t, err) + expectEquals(t, groups, []string{ + "org-1:team-1", + "org-1:team-2", + "org-1:team-3", + "org-2:team-4", + "org-3", + }) + + s.Close() +} + +func TestUserGroupsWithoutOrgs(t *testing.T) { + + s := newTestServer(map[string]interface{}{ + "/user/orgs": []org{}, + "/user/teams": []team{}, + }) + + connector := githubConnector{apiURL: s.URL} + groups, err := connector.userGroups(context.Background(), newClient()) + + expectNil(t, err) + expectEquals(t, len(groups), 0) + + s.Close() +} + +func TestUsernameIncludedInFederatedIdentity(t *testing.T) { + + s := newTestServer(map[string]interface{}{ + "/user": user{Login: "some-login"}, + "/user/emails": []userEmail{{ + Email: "some@email.com", + Verified: true, + Primary: true, + }}, + "/login/oauth/access_token": map[string]interface{}{ + "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", + "expires_in": "30", + }, + }) + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + req, err := http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + githubConnector := githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient()} + identity, err := githubConnector.HandleCallback(connector.Scopes{}, req) + + expectNil(t, err) + expectEquals(t, identity.Username, "some-login") + + s.Close() +} + +func newTestServer(responses map[string]interface{}) *httptest.Server { + return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + json.NewEncoder(w).Encode(responses[r.URL.Path]) + })) +} + +func newClient() *http.Client { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + return &http.Client{Transport: tr} +} + +func expectNil(t *testing.T, a interface{}) { + if a != nil { + t.Errorf("Expected %+v to equal nil", a) + } +} + +func expectEquals(t *testing.T, a interface{}, b interface{}) { + if !reflect.DeepEqual(a, b) { + t.Errorf("Expected %+v to equal %+v", a, b) + } +} From e10b8232d1a368bdaf7b645e776050f677dba27c Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 15 Nov 2018 08:12:28 -0800 Subject: [PATCH 2/7] Apply reviewer notes: style changes, make sure unit test verifies pagination --- connector/github/github.go | 35 ++++++----- connector/github/github_test.go | 104 ++++++++++++++++++++------------ 2 files changed, 85 insertions(+), 54 deletions(-) diff --git a/connector/github/github.go b/connector/github/github.go index edb821ea..123e88ba 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -331,7 +331,8 @@ func (c *githubConnector) getGroups(ctx context.Context, client *http.Client, gr return nil, nil } -// formatTeamName return unique team name: prgs might have the same team names team name should be prefixed with org name to make team names unique across orgs. +// formatTeamName returns unique team name. +// Orgs might have the same team names. To make team name unique it should be prefixed with the org name. func formatTeamName(org string, team string) string { return fmt.Sprintf("%s:%s", org, team) } @@ -342,12 +343,13 @@ func formatTeamName(org string, team string) string { // N orgs, M teams per org: user is member of any team from at least 1 org // N-1 orgs, M teams per org, 1 org with no teams: user is member of any team // from at least 1 org, or member of org with no teams -func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client, userName string) (groups []string, err error) { +func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client, userName string) ([]string, error) { + groups := make([]string, 0) var inOrgNoTeams bool for _, org := range c.orgs { inOrg, err := c.userInOrg(ctx, client, userName, org.Name) if err != nil { - return groups, err + return nil, err } if !inOrg { continue @@ -355,7 +357,7 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client teams, err := c.teamsForOrg(ctx, client, org.Name) if err != nil { - return groups, err + return nil, err } // User is in at least one org. User is authorized if no teams are specified // in config; include all teams in claim. Otherwise filter out teams not in @@ -371,22 +373,23 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client } } if inOrgNoTeams || len(groups) > 0 { - return + return groups, nil } return groups, fmt.Errorf("github: user %q not in required orgs or teams", userName) } -func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) (groups []string, err error) { +func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) ([]string, error) { orgs, err := c.userOrgs(ctx, client) if err != nil { - return groups, err + return nil, err } orgTeams, err := c.userOrgTeams(ctx, client) if err != nil { - return groups, err + return nil, err } + groups := make([]string, 0) for _, org := range orgs { if teams, ok := orgTeams[org]; !ok { groups = append(groups, org) @@ -397,12 +400,13 @@ func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) ( } } - return groups, err + return groups, nil } // userOrgs retrieves list of current user orgs func (c *githubConnector) userOrgs(ctx context.Context, client *http.Client) ([]string, error) { - apiURL, groups := c.apiURL+"/user/orgs", []string{} + groups := make([]string, 0) + apiURL := c.apiURL + "/user/orgs" for { // https://developer.github.com/v3/orgs/#list-your-organizations var ( @@ -413,8 +417,8 @@ func (c *githubConnector) userOrgs(ctx context.Context, client *http.Client) ([] return nil, fmt.Errorf("github: get orgs: %v", err) } - for _, org := range orgs { - groups = append(groups, org.Login) + for _, o := range orgs { + groups = append(groups, o.Login) } if apiURL == "" { @@ -428,7 +432,8 @@ func (c *githubConnector) userOrgs(ctx context.Context, client *http.Client) ([] // userOrgTeams retrieves teams which current user belongs to. // Method returns a map where key is an org name and value list of teams under the org. func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) (map[string][]string, error) { - apiURL, groups := c.apiURL+"/user/teams", map[string][]string{} + groups := make(map[string][]string, 0) + apiURL := c.apiURL + "/user/teams" for { // https://developer.github.com/v3/orgs/teams/#list-user-teams var ( @@ -439,8 +444,8 @@ func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) return nil, fmt.Errorf("github: get teams: %v", err) } - for _, team := range teams { - groups[team.Org.Login] = append(groups[team.Org.Login], team.Name) + for _, t := range teams { + groups[t.Org.Login] = append(groups[t.Org.Login], t.Name) } if apiURL == "" { diff --git a/connector/github/github_test.go b/connector/github/github_test.go index 57aef554..44519a6e 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -4,37 +4,52 @@ import ( "context" "crypto/tls" "encoding/json" + "fmt" "net/http" "net/http/httptest" "net/url" "reflect" + "strings" "testing" "github.com/dexidp/dex/connector" ) +type testResponse struct { + data interface{} + nextLink string + lastLink string +} + func TestUserGroups(t *testing.T) { - - orgs := []org{ - {Login: "org-1"}, - {Login: "org-2"}, - {Login: "org-3"}, - } - - teams := []team{ - {Name: "team-1", Org: org{Login: "org-1"}}, - {Name: "team-2", Org: org{Login: "org-1"}}, - {Name: "team-3", Org: org{Login: "org-1"}}, - {Name: "team-4", Org: org{Login: "org-2"}}, - } - - s := newTestServer(map[string]interface{}{ - "/user/orgs": orgs, - "/user/teams": teams, + s := newTestServer(map[string]testResponse{ + "/user/orgs": { + data: []org{{Login: "org-1"}, {Login: "org-2"}}, + nextLink: "/user/orgs?since=2", + lastLink: "/user/orgs?since=2", + }, + "/user/orgs?since=2": {data: []org{{Login: "org-3"}}}, + "/user/teams": { + data: []team{ + {Name: "team-1", Org: org{Login: "org-1"}}, + {Name: "team-2", Org: org{Login: "org-1"}}, + }, + nextLink: "/user/teams?since=2", + lastLink: "/user/teams?since=2", + }, + "/user/teams?since=2": { + data: []team{ + {Name: "team-3", Org: org{Login: "org-1"}}, + {Name: "team-4", Org: org{Login: "org-2"}}, + }, + nextLink: "/user/teams?since=2", + lastLink: "/user/teams?since=2", + }, }) + defer s.Close() - connector := githubConnector{apiURL: s.URL} - groups, err := connector.userGroups(context.Background(), newClient()) + c := githubConnector{apiURL: s.URL} + groups, err := c.userGroups(context.Background(), newClient()) expectNil(t, err) expectEquals(t, groups, []string{ @@ -44,40 +59,39 @@ func TestUserGroups(t *testing.T) { "org-2:team-4", "org-3", }) - - s.Close() } func TestUserGroupsWithoutOrgs(t *testing.T) { - s := newTestServer(map[string]interface{}{ - "/user/orgs": []org{}, - "/user/teams": []team{}, + s := newTestServer(map[string]testResponse{ + "/user/orgs": {data: []org{}}, + "/user/teams": {data: []team{}}, }) + defer s.Close() - connector := githubConnector{apiURL: s.URL} - groups, err := connector.userGroups(context.Background(), newClient()) + c := githubConnector{apiURL: s.URL} + groups, err := c.userGroups(context.Background(), newClient()) expectNil(t, err) expectEquals(t, len(groups), 0) - s.Close() } func TestUsernameIncludedInFederatedIdentity(t *testing.T) { - s := newTestServer(map[string]interface{}{ - "/user": user{Login: "some-login"}, - "/user/emails": []userEmail{{ + s := newTestServer(map[string]testResponse{ + "/user": {data: user{Login: "some-login"}}, + "/user/emails": {data: []userEmail{{ Email: "some@email.com", Verified: true, Primary: true, - }}, - "/login/oauth/access_token": map[string]interface{}{ + }}}, + "/login/oauth/access_token": {data: map[string]interface{}{ "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", "expires_in": "30", - }, + }}, }) + defer s.Close() hostURL, err := url.Parse(s.URL) expectNil(t, err) @@ -85,20 +99,32 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) { req, err := http.NewRequest("GET", hostURL.String(), nil) expectNil(t, err) - githubConnector := githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient()} - identity, err := githubConnector.HandleCallback(connector.Scopes{}, req) + c := githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient()} + identity, err := c.HandleCallback(connector.Scopes{}, req) expectNil(t, err) expectEquals(t, identity.Username, "some-login") - s.Close() } -func newTestServer(responses map[string]interface{}) *httptest.Server { - return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +func newTestServer(responses map[string]testResponse) *httptest.Server { + var s *httptest.Server + s = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + response := responses[r.RequestURI] + linkParts := make([]string, 0) + if response.nextLink != "" { + linkParts = append(linkParts, fmt.Sprintf("<%s%s>; rel=\"next\"", s.URL, response.nextLink)) + } + if response.lastLink != "" { + linkParts = append(linkParts, fmt.Sprintf("<%s%s>; rel=\"last\"", s.URL, response.lastLink)) + } + if len(linkParts) > 0 { + w.Header().Add("Link", strings.Join(linkParts, ", ")) + } w.Header().Add("Content-Type", "application/json") - json.NewEncoder(w).Encode(responses[r.URL.Path]) + json.NewEncoder(w).Encode(response.data) })) + return s } func newClient() *http.Client { From a9f71e378f26ff4dd81a1e4f0ae9e44162f85394 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 15 Nov 2018 08:57:31 -0800 Subject: [PATCH 3/7] Update getPagination method comment --- connector/github/github.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/connector/github/github.go b/connector/github/github.go index 123e88ba..8399e8d2 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -503,10 +503,10 @@ func get(ctx context.Context, client *http.Client, apiURL string, v interface{}) return getPagination(apiURL, resp), nil } -// getPagination checks the "Link" header field for "next" or "last" pagination -// URLs, and returns true only if a "next" URL is found. The next pages' URL is -// returned if a "next" URL is found. apiURL is returned if apiURL equals the -// "last" URL or no "next" or "last" URL are found. +// getPagination checks the "Link" header field for "next" or "last" pagination URLs, +// and returns "next" page URL or empty string to indicate that there are no more pages. +// Non empty next pages' URL is returned if both "last" and "next" URLs are found and next page +// URL is not equal to last. // // https://developer.github.com/v3/#pagination func getPagination(apiURL string, resp *http.Response) string { From e8763531287a18ab31a0eb3776916e1d04893d4c Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 15 Nov 2018 09:00:37 -0800 Subject: [PATCH 4/7] Rename variables to stop shadowing package name --- connector/github/github.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/connector/github/github.go b/connector/github/github.go index 8399e8d2..d28cb096 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -390,12 +390,12 @@ func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) ( } groups := make([]string, 0) - for _, org := range orgs { - if teams, ok := orgTeams[org]; !ok { - groups = append(groups, org) + for _, o := range orgs { + if teams, ok := orgTeams[o]; !ok { + groups = append(groups, o) } else { - for _, team := range teams { - groups = append(groups, formatTeamName(org, team)) + for _, t := range teams { + groups = append(groups, formatTeamName(o, t)) } } } From ce3cd53a11081782f907ab6a3346f81357a20326 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 15 Nov 2018 09:23:57 -0800 Subject: [PATCH 5/7] Bug fix: take into account 'teamNameField' settings while fetching all user groups --- connector/github/github.go | 23 ++++++++++++++--------- connector/github/github_test.go | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/connector/github/github.go b/connector/github/github.go index d28cb096..5e9a302b 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -445,7 +445,7 @@ func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) } for _, t := range teams { - groups[t.Org.Login] = append(groups[t.Org.Login], t.Name) + groups[t.Org.Login] = append(groups[t.Org.Login], c.teamGroupClaim(t)) } if apiURL == "" { @@ -680,14 +680,9 @@ func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client, return nil, fmt.Errorf("github: get teams: %v", err) } - for _, team := range teams { - if team.Org.Login == orgName { - switch c.teamNameField { - case "name", "": - groups = append(groups, team.Name) - case "slug": - groups = append(groups, team.Slug) - } + for _, t := range teams { + if t.Org.Login == orgName { + groups = append(groups, c.teamGroupClaim(t)) } } @@ -698,3 +693,13 @@ func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client, return groups, nil } + +// teamGroupClaim returns team slag if 'teamNameField; option is set to 'slug' otherwise returns team name. +func (c *githubConnector) teamGroupClaim(t team) string { + switch c.teamNameField { + case "slug": + return t.Slug + default: + return t.Name + } +} diff --git a/connector/github/github_test.go b/connector/github/github_test.go index 44519a6e..e871c607 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -77,6 +77,28 @@ func TestUserGroupsWithoutOrgs(t *testing.T) { } +func TestUserGroupsWithTeamNameFieldConfig(t *testing.T) { + s := newTestServer(map[string]testResponse{ + "/user/orgs": { + data: []org{{Login: "org-1"}}, + }, + "/user/teams": { + data: []team{ + {Name: "Team 1", Slug: "team-1", Org: org{Login: "org-1"}}, + }, + }, + }) + defer s.Close() + + c := githubConnector{apiURL: s.URL, teamNameField: "slug"} + groups, err := c.userGroups(context.Background(), newClient()) + + expectNil(t, err) + expectEquals(t, groups, []string{ + "org-1:team-1", + }) +} + func TestUsernameIncludedInFederatedIdentity(t *testing.T) { s := newTestServer(map[string]testResponse{ From e5ebcf518a9999a811d77ad77ccbc5793bcaf5e0 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 15 Nov 2018 09:24:21 -0800 Subject: [PATCH 6/7] Update github connector documentation --- Documentation/connectors/github.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/connectors/github.md b/Documentation/connectors/github.md index c07ac4c3..ddf11869 100644 --- a/Documentation/connectors/github.md +++ b/Documentation/connectors/github.md @@ -42,8 +42,12 @@ connectors: # For example if a user is part of the "engineering" team of the "coreos" # org, the group claim would include "coreos:engineering". # - # A user MUST be a member of at least one of the following orgs to + # 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: + # provide read-only access to everyone and give full permissions if user has 'my-organization:admins-team' group claim. orgs: - name: my-organization # Include all teams as claims. From 20bc6cd35379be5bd5b17cd473093c39ab826739 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 15 Nov 2018 14:08:15 -0800 Subject: [PATCH 7/7] Full list of groups should include group names as well as group_name:team_name --- connector/github/github.go | 5 ++--- connector/github/github_test.go | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/connector/github/github.go b/connector/github/github.go index 5e9a302b..977d190f 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -391,9 +391,8 @@ func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) ( groups := make([]string, 0) for _, o := range orgs { - if teams, ok := orgTeams[o]; !ok { - groups = append(groups, o) - } else { + groups = append(groups, o) + if teams, ok := orgTeams[o]; ok { for _, t := range teams { groups = append(groups, formatTeamName(o, t)) } diff --git a/connector/github/github_test.go b/connector/github/github_test.go index e871c607..7069091d 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -53,9 +53,11 @@ func TestUserGroups(t *testing.T) { expectNil(t, err) expectEquals(t, groups, []string{ + "org-1", "org-1:team-1", "org-1:team-2", "org-1:team-3", + "org-2", "org-2:team-4", "org-3", }) @@ -95,6 +97,7 @@ func TestUserGroupsWithTeamNameFieldConfig(t *testing.T) { expectNil(t, err) expectEquals(t, groups, []string{ + "org-1", "org-1:team-1", }) }