Modernize role mapping in github connectors (#10456)

api/types/github.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/utils"
+	log "github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
 
 	"github.com/gravitational/trace"
@@ -48,9 +49,13 @@ type GithubConnector interface {
 	GetTeamsToLogins() []TeamMapping
 	// SetTeamsToLogins sets the mapping of Github teams to allowed logins
 	SetTeamsToLogins([]TeamMapping)
+	// GetTeamsToRoles returns the mapping of Github teams to allowed roles
+	GetTeamsToRoles() []TeamRolesMapping
+	// SetTeamsToRoles sets the mapping of Github teams to allowed roles
+	SetTeamsToRoles([]TeamRolesMapping)
 	// MapClaims returns the list of allows logins based on the retrieved claims
 	// returns list of logins and kubernetes groups
-	MapClaims(GithubClaims) (logins []string, kubeGroups []string, kubeUsers []string)
+	MapClaims(GithubClaims) (roles []string, kubeGroups []string, kubeUsers []string)
 	// GetDisplay returns the connector display name
 	GetDisplay() string
 	// SetDisplay sets the connector display name
@@ -154,15 +159,25 @@ func (c *GithubConnectorV3) CheckAndSetDefaults() error {
 		return trace.Wrap(err)
 	}
 
+	// DELETE IN 11.0.0
+	if len(c.Spec.TeamsToLogins) > 0 {
+		log.Warn("GitHub connector field teams_to_logins is deprecated and will be removed in the next version. Please use teams_to_roles instead.")
+	}
+
 	// make sure claim mappings have either roles or a role template
 	for i, v := range c.Spec.TeamsToLogins {
 		if v.Team == "" {
 			return trace.BadParameter("team_to_logins mapping #%v is invalid, team is empty.", i+1)
 		}
 	}
+	for i, v := range c.Spec.TeamsToRoles {
+		if v.Team == "" {
+			return trace.BadParameter("team_to_roles mapping #%v is invalid, team is empty.", i+1)
+		}
+	}
 
-	if len(c.Spec.TeamsToLogins) == 0 {
-		return trace.BadParameter("team_to_logins mapping is invalid, no mappings defined.")
+	if len(c.Spec.TeamsToLogins)+len(c.Spec.TeamsToRoles) == 0 {
+		return trace.BadParameter("team_to_logins or team_to_roles mapping is invalid, no mappings defined.")
 	}
 
 	return nil
@@ -199,15 +214,29 @@ func (c *GithubConnectorV3) SetRedirectURL(redirectURL string) {
 }
 
 // GetTeamsToLogins returns the connector team membership mappings
+//
+// DEPRECATED: use GetTeamsToRoles instead
 func (c *GithubConnectorV3) GetTeamsToLogins() []TeamMapping {
 	return c.Spec.TeamsToLogins
 }
 
 // SetTeamsToLogins sets the connector team membership mappings
+//
+// DEPRECATED: use SetTeamsToRoles instead
 func (c *GithubConnectorV3) SetTeamsToLogins(teamsToLogins []TeamMapping) {
 	c.Spec.TeamsToLogins = teamsToLogins
 }
 
+// GetTeamsToRoles returns the mapping of Github teams to allowed roles
+func (c *GithubConnectorV3) GetTeamsToRoles() []TeamRolesMapping {
+	return c.Spec.TeamsToRoles
+}
+
+// SetTeamsToRoles sets the mapping of Github teams to allowed roles
+func (c *GithubConnectorV3) SetTeamsToRoles(m []TeamRolesMapping) {
+	c.Spec.TeamsToRoles = m
+}
+
 // GetDisplay returns the connector display name
 func (c *GithubConnectorV3) GetDisplay() string {
 	return c.Spec.Display
@@ -221,7 +250,7 @@ func (c *GithubConnectorV3) SetDisplay(display string) {
 // MapClaims returns a list of logins based on the provided claims,
 // returns a list of logins and list of kubernetes groups
 func (c *GithubConnectorV3) MapClaims(claims GithubClaims) ([]string, []string, []string) {
-	var logins, kubeGroups, kubeUsers []string
+	var roles, kubeGroups, kubeUsers []string
 	for _, mapping := range c.GetTeamsToLogins() {
 		teams, ok := claims.OrganizationToTeams[mapping.Organization]
 		if !ok {
@@ -231,13 +260,26 @@ func (c *GithubConnectorV3) MapClaims(claims GithubClaims) ([]string, []string,
 		for _, team := range teams {
 			// see if the user belongs to this team
 			if team == mapping.Team {
-				logins = append(logins, mapping.Logins...)
+				roles = append(roles, mapping.Logins...)
 				kubeGroups = append(kubeGroups, mapping.KubeGroups...)
 				kubeUsers = append(kubeUsers, mapping.KubeUsers...)
 			}
 		}
 	}
-	return utils.Deduplicate(logins), utils.Deduplicate(kubeGroups), utils.Deduplicate(kubeUsers)
+	for _, mapping := range c.GetTeamsToRoles() {
+		teams, ok := claims.OrganizationToTeams[mapping.Organization]
+		if !ok {
+			// the user does not belong to this organization
+			continue
+		}
+		for _, team := range teams {
+			// see if the user belongs to this team
+			if team == mapping.Team {
+				roles = append(roles, mapping.Roles...)
+			}
+		}
+	}
+	return utils.Deduplicate(roles), utils.Deduplicate(kubeGroups), utils.Deduplicate(kubeUsers)
 }
 
 // SetExpiry sets expiry time for the object

api/types/types.proto

@@ -2877,10 +2877,16 @@ message GithubConnectorSpecV3 {
     // RedirectURL is the authorization callback URL.
     string RedirectURL = 3 [ (gogoproto.jsontag) = "redirect_url" ];
     // TeamsToLogins maps Github team memberships onto allowed logins/roles.
+    //
+    // DELETE IN 11.0.0
+    // Deprecated: use GithubTeamsToRoles instead.
     repeated TeamMapping TeamsToLogins = 4
         [ (gogoproto.nullable) = false, (gogoproto.jsontag) = "teams_to_logins" ];
     // Display is the connector display name.
     string Display = 5 [ (gogoproto.jsontag) = "display" ];
+    // TeamsToRoles maps Github team memberships onto allowed roles.
+    repeated TeamRolesMapping TeamsToRoles = 6
+        [ (gogoproto.nullable) = false, (gogoproto.jsontag) = "teams_to_roles" ];
 }
 
 // GithubAuthRequest is the request to start Github OAuth2 flow.
@@ -3081,6 +3087,8 @@ message GithubClaims {
 }
 
 // TeamMapping represents a single team membership mapping.
+//
+// DELETE IN 11.0.0
 message TeamMapping {
     // Organization is a Github organization a user belongs to.
     string Organization = 1 [ (gogoproto.jsontag) = "organization" ];
@@ -3094,6 +3102,16 @@ message TeamMapping {
     repeated string KubeUsers = 5 [ (gogoproto.jsontag) = "kubernetes_users,omitempty" ];
 }
 
+// TeamRolesMapping represents a single team membership mapping.
+message TeamRolesMapping {
+    // Organization is a Github organization a user belongs to.
+    string Organization = 1 [ (gogoproto.jsontag) = "organization" ];
+    // Team is a team within the organization a user belongs to.
+    string Team = 2 [ (gogoproto.jsontag) = "team" ];
+    // Roles is a list of allowed logins for this org/team.
+    repeated string Roles = 3 [ (gogoproto.jsontag) = "roles,omitempty" ];
+}
+
 // TrustedClusterV2 represents a Trusted Cluster.
 message TrustedClusterV2 {
     option (gogoproto.goproto_stringer) = false;

lib/auth/github.go

@@ -352,7 +352,6 @@ func (a *Server) validateGithubAuthCallback(ctx context.Context, diagCtx *ssoDia
 	diagCtx.info.CreateUserParams = &types.CreateUserParams{
 		ConnectorName: params.connectorName,
 		Username:      params.username,
-		Logins:        params.logins,
 		KubeGroups:    params.kubeGroups,
 		KubeUsers:     params.kubeUsers,
 		Roles:         params.roles,
@@ -435,9 +434,6 @@ type createUserParams struct {
 	// username is the Teleport user name .
 	username string
 
-	// logins is the list of *nix logins.
-	logins []string
-
 	// kubeGroups is the list of Kubernetes groups this user belongs to.
 	kubeGroups []string
 
@@ -461,13 +457,12 @@ func (a *Server) calculateGithubUser(connector types.GithubConnector, claims *ty
 	}
 
 	// Calculate logins, kubegroups, roles, and traits.
-	p.logins, p.kubeGroups, p.kubeUsers = connector.MapClaims(*claims)
-	if len(p.logins) == 0 {
+	p.roles, p.kubeGroups, p.kubeUsers = connector.MapClaims(*claims)
+	if len(p.roles) == 0 {
 		return nil, trace.BadParameter(
 			"user %q does not belong to any teams configured in %q connector; the configuration may have typos.",
 			claims.Username, connector.GetName())
 	}
-	p.roles = p.logins
 	p.traits = map[string][]string{
 		teleport.TraitLogins:     {p.username},
 		teleport.TraitKubeGroups: p.kubeGroups,
@@ -488,8 +483,8 @@ func (a *Server) calculateGithubUser(connector types.GithubConnector, claims *ty
 
 func (a *Server) createGithubUser(ctx context.Context, p *createUserParams, dryRun bool) (types.User, error) {
 	log.WithFields(logrus.Fields{trace.Component: "github"}).Debugf(
-		"Generating dynamic GitHub identity %v/%v with logins: %v. Dry run: %v.",
-		p.connectorName, p.username, p.logins, dryRun)
+		"Generating dynamic GitHub identity %v/%v with roles: %v. Dry run: %v.",
+		p.connectorName, p.username, p.roles, dryRun)
 
 	expires := a.GetClock().Now().UTC().Add(p.sessionTTL)
 

lib/auth/github_test.go

@@ -95,7 +95,6 @@ func (s *GithubSuite) TestCreateGithubUser(c *check.C) {
 	user, err := s.a.createGithubUser(context.Background(), &createUserParams{
 		connectorName: "github",
 		username:      "foo@example.com",
-		logins:        []string{"foo"},
 		roles:         []string{"admin"},
 		sessionTTL:    1 * time.Minute,
 	}, true)
@@ -110,7 +109,6 @@ func (s *GithubSuite) TestCreateGithubUser(c *check.C) {
 	_, err = s.a.createGithubUser(context.Background(), &createUserParams{
 		connectorName: "github",
 		username:      "foo",
-		logins:        []string{"foo"},
 		roles:         []string{"admin"},
 		sessionTTL:    1 * time.Minute,
 	}, false)

lib/auth/oidc.go

@@ -489,7 +489,6 @@ func (a *Server) validateOIDCAuthCallback(ctx context.Context, diagCtx *ssoDiagC
 	diagCtx.info.CreateUserParams = &types.CreateUserParams{
 		ConnectorName: params.connectorName,
 		Username:      params.username,
-		Logins:        params.logins,
 		KubeGroups:    params.kubeGroups,
 		KubeUsers:     params.kubeUsers,
 		Roles:         params.roles,

lib/auth/oidc_test.go

@@ -106,7 +106,6 @@ func TestCreateOIDCUser(t *testing.T) {
 	user, err := s.a.createOIDCUser(&createUserParams{
 		connectorName: "oidcService",
 		username:      "foo@example.com",
-		logins:        []string{"foo"},
 		roles:         []string{"admin"},
 		sessionTTL:    1 * time.Minute,
 	}, true)
@@ -121,7 +120,6 @@ func TestCreateOIDCUser(t *testing.T) {
 	_, err = s.a.createOIDCUser(&createUserParams{
 		connectorName: "oidcService",
 		username:      "foo@example.com",
-		logins:        []string{"foo"},
 		roles:         []string{"admin"},
 		sessionTTL:    1 * time.Minute,
 	}, false)

lib/auth/saml.go

@@ -482,7 +482,6 @@ func (a *Server) validateSAMLResponse(ctx context.Context, diagCtx *ssoDiagConte
 	diagCtx.info.CreateUserParams = &types.CreateUserParams{
 		ConnectorName: params.connectorName,
 		Username:      params.username,
-		Logins:        params.logins,
 		KubeGroups:    params.kubeGroups,
 		KubeUsers:     params.kubeUsers,
 		Roles:         params.roles,

lib/auth/saml_test.go

@@ -71,7 +71,6 @@ func TestCreateSAMLUser(t *testing.T) {
 	user, err := a.createSAMLUser(&createUserParams{
 		connectorName: "samlService",
 		username:      "foo@example.com",
-		logins:        []string{"foo"},
 		roles:         []string{"admin"},
 		sessionTTL:    1 * time.Minute,
 	}, true)
@@ -86,7 +85,6 @@ func TestCreateSAMLUser(t *testing.T) {
 	_, err = a.createSAMLUser(&createUserParams{
 		connectorName: "samlService",
 		username:      "foo@example.com",
-		logins:        []string{"foo"},
 		roles:         []string{"admin"},
 		sessionTTL:    1 * time.Minute,
 	}, false)

lib/services/github_test.go

@@ -82,33 +82,41 @@ func (g *GithubSuite) TestMapClaims(c *check.C) {
 				KubeGroups:   []string{"kube-devs"},
 			},
 		},
+		TeamsToRoles: []types.TeamRolesMapping{
+			{
+				Organization: "gravitational",
+				Team:         "admins",
+				Roles:        []string{"system"},
+			},
+		},
 	})
 	c.Assert(err, check.IsNil)
 
-	logins, kubeGroups, kubeUsers := connector.MapClaims(types.GithubClaims{
+	roles, kubeGroups, kubeUsers := connector.MapClaims(types.GithubClaims{
 		OrganizationToTeams: map[string][]string{
 			"gravitational": {"admins"},
 		},
 	})
-	c.Assert(logins, check.DeepEquals, []string{"admin", "dev"})
+	c.Assert(roles, check.DeepEquals, []string{"admin", "dev", "system"})
 	c.Assert(kubeGroups, check.DeepEquals, []string{"system:masters", "kube-devs"})
 	c.Assert(kubeUsers, check.DeepEquals, []string{"alice@example.com"})
 
-	logins, kubeGroups, kubeUsers = connector.MapClaims(types.GithubClaims{
+	roles, kubeGroups, kubeUsers = connector.MapClaims(types.GithubClaims{
 		OrganizationToTeams: map[string][]string{
 			"gravitational": {"devs"},
 		},
 	})
-	c.Assert(logins, check.DeepEquals, []string{"dev", "test"})
+
+	c.Assert(roles, check.DeepEquals, []string{"dev", "test"})
 	c.Assert(kubeGroups, check.DeepEquals, []string{"kube-devs"})
 	c.Assert(kubeUsers, check.DeepEquals, []string(nil))
 
-	logins, kubeGroups, kubeUsers = connector.MapClaims(types.GithubClaims{
+	roles, kubeGroups, kubeUsers = connector.MapClaims(types.GithubClaims{
 		OrganizationToTeams: map[string][]string{
 			"gravitational": {"admins", "devs"},
 		},
 	})
-	c.Assert(logins, check.DeepEquals, []string{"admin", "dev", "test"})
+	c.Assert(roles, check.DeepEquals, []string{"admin", "dev", "test", "system"})
 	c.Assert(kubeGroups, check.DeepEquals, []string{"system:masters", "kube-devs"})
 	c.Assert(kubeUsers, check.DeepEquals, []string{"alice@example.com"})
 }

lib/web/resources_test.go

@@ -85,6 +85,7 @@ spec:
     - dummy
     organization: octocats
     team: dummy
+  teams_to_roles: null
 version: v3
 `
 	githubConn, err := types.NewGithubConnector("githubName", types.GithubConnectorSpecV3{