Merge pull request #1345 from concourse/pr/github-team-name-and-slug

Add 'both' option to use team name AND slug in TeamNameField

This allows the connector to be configured with both so that both team names and slugs can be returned in the groups. This makes configuring teams in an application a bit more foolproof; we would often have confusion over whether the team name or slug should be given by the user, so it's easier to just allow both, since collisions shouldn't be possible anyway.

Documentation/connectors/github.md

@@ -59,12 +59,13 @@ connectors:
     # Flag which indicates that all user groups and teams should be loaded.
     loadAllGroups: false
 
-    # Optional choice between 'name' (default) or 'slug'.
+    # Optional choice between 'name' (default), 'slug', or 'both'.
     #
     # As an example, group claims for member of 'Site Reliability Engineers' in
     # Acme organization would yield:
     #   - ['acme:Site Reliability Engineers'] for 'name'
     #   - ['acme:site-reliability-engineers'] for 'slug'
+    #   - ['acme:Site Reliability Engineers', 'acme:site-reliability-engineers'] for 'both'
     teamNameField: slug
 ```
 

connector/github/github.go

@@ -16,11 +16,10 @@ import (
 	"strings"
 	"time"
 
+	"github.com/sirupsen/logrus"
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/github"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/dexidp/dex/connector"
 )
 
@@ -111,7 +110,7 @@ func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector
 	g.loadAllGroups = c.LoadAllGroups
 
 	switch c.TeamNameField {
-	case "name", "slug", "":
+	case "name", "slug", "both", "":
 		g.teamNameField = c.TeamNameField
 	default:
 		return nil, fmt.Errorf("invalid connector config: unsupported team name field value `%s`", c.TeamNameField)
@@ -449,7 +448,7 @@ func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client)
 		}
 
 		for _, t := range teams {
-			groups[t.Org.Login] = append(groups[t.Org.Login], c.teamGroupClaim(t))
+			groups[t.Org.Login] = append(groups[t.Org.Login], c.teamGroupClaims(t)...)
 		}
 
 		if apiURL == "" {
@@ -686,7 +685,7 @@ func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client,
 
 		for _, t := range teams {
 			if t.Org.Login == orgName {
-				groups = append(groups, c.teamGroupClaim(t))
+				groups = append(groups, c.teamGroupClaims(t)...)
 			}
 		}
 
@@ -698,12 +697,16 @@ 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 {
+// teamGroupClaims returns team slug if 'teamNameField' option is set to
+// 'slug', returns the slug *and* name if set to 'both', otherwise returns team
+// name.
+func (c *githubConnector) teamGroupClaims(t team) []string {
 	switch c.teamNameField {
+	case "both":
+		return []string{t.Name, t.Slug}
 	case "slug":
-		return t.Slug
+		return []string{t.Slug}
 	default:
-		return t.Name
+		return []string{t.Name}
 	}
 }

connector/github/github_test.go

@@ -64,7 +64,6 @@ func TestUserGroups(t *testing.T) {
 }
 
 func TestUserGroupsWithoutOrgs(t *testing.T) {
-
 	s := newTestServer(map[string]testResponse{
 		"/user/orgs":  {data: []org{}},
 		"/user/teams": {data: []team{}},
@@ -76,7 +75,6 @@ func TestUserGroupsWithoutOrgs(t *testing.T) {
 
 	expectNil(t, err)
 	expectEquals(t, len(groups), 0)
-
 }
 
 func TestUserGroupsWithTeamNameFieldConfig(t *testing.T) {
@@ -102,6 +100,30 @@ func TestUserGroupsWithTeamNameFieldConfig(t *testing.T) {
 	})
 }
 
+func TestUserGroupsWithTeamNameAndSlugFieldConfig(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: "both"}
+	groups, err := c.userGroups(context.Background(), newClient())
+
+	expectNil(t, err)
+	expectEquals(t, groups, []string{
+		"org-1",
+		"org-1:Team 1",
+		"org-1:team-1",
+	})
+}
+
 func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
 
 	s := newTestServer(map[string]testResponse{