Consolidate external identity user creation code.

lib/auth/github.go

@@ -150,56 +150,48 @@ func (s *AuthServer) validateGithubAuthCallback(q url.Values) (*GithubAuthRespon
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	expires := s.clock.Now().UTC().Add(defaults.OAuth2TTL)
-	err = s.createGithubUser(connector, *claims, expires)
+
+	// Calculate (figure out name, roles, traits, session TTL) of user and
+	// create the user in the backend.
+	params, err := s.calculateGithubUser(connector, claims, req)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	user, err := s.createGithubUser(params)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// Auth was successful, return session, certificate, etc. to caller.
 	response := &GithubAuthResponse{
+		Req: *req,
 		Identity: services.ExternalIdentity{
-			ConnectorID: connector.GetName(),
-			Username:    claims.Username,
+			ConnectorID: params.connectorName,
+			Username:    params.username,
 		},
-		Req: *req,
-	}
-	user, err := s.Identity.GetUserByGithubIdentity(response.Identity)
-	if err != nil {
-		return nil, trace.Wrap(err)
+		Username: user.GetName(),
 	}
 	response.Username = user.GetName()
-	roles, err := services.FetchRoles(user.GetRoles(), s.Access, user.GetTraits())
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
+
+	// If the request is coming from a browser, create a web session.
 	if req.CreateWebSession {
-		session, err := s.NewWebSession(user.GetName())
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
-		sessionTTL := roles.AdjustSessionTTL(defaults.OAuth2TTL)
-		bearerTTL := utils.MinTTL(BearerTokenTTL, sessionTTL)
-		session.SetExpiryTime(s.clock.Now().UTC().Add(sessionTTL))
-		session.SetBearerTokenExpiryTime(s.clock.Now().UTC().Add(bearerTTL))
-		err = s.UpsertWebSession(user.GetName(), session)
+		session, err := s.createWebSession(user, params.sessionTTL)
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
+
 		response.Session = session
 	}
+
+	// If a public key was provided, sign it and return a certificate.
 	if len(req.PublicKey) != 0 {
-		certTTL := utils.MinTTL(defaults.OAuth2TTL, req.CertTTL)
-		certs, err := s.generateUserCert(certRequest{
-			user:          user,
-			roles:         roles,
-			ttl:           certTTL,
-			publicKey:     req.PublicKey,
-			compatibility: req.Compatibility,
-		})
+		sshCert, tlsCert, err := s.createSessionCert(user, params.sessionTTL, req.PublicKey, req.Compatibility)
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
-		response.Cert = certs.ssh
-		response.TLSCert = certs.tls
+
+		response.Cert = sshCert
+		response.TLSCert = tlsCert
 
 		// Return the host CA for this cluster only.
 		authority, err := s.GetCertAuthority(services.CertAuthID{
@@ -211,61 +203,157 @@ func (s *AuthServer) validateGithubAuthCallback(q url.Values) (*GithubAuthRespon
 		}
 		response.HostSigners = append(response.HostSigners, authority)
 	}
+
 	return response, nil
 }
 
-func (s *AuthServer) createGithubUser(connector services.GithubConnector, claims services.GithubClaims, expires time.Time) error {
-	logins, kubeGroups := connector.MapClaims(claims)
-	if len(logins) == 0 {
-		return trace.BadParameter(
+func (s *AuthServer) createWebSession(user services.User, sessionTTL time.Duration) (services.WebSession, error) {
+	session, err := s.NewWebSession(user.GetName())
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// Session expiry time is the same as the user expiry time.
+	session.SetExpiryTime(s.clock.Now().UTC().Add(sessionTTL))
+
+	// Bearer tokens expire quicker than the overall session time and need to be refreshed.
+	bearerTTL := utils.MinTTL(BearerTokenTTL, sessionTTL)
+	session.SetBearerTokenExpiryTime(s.clock.Now().UTC().Add(bearerTTL))
+
+	err = s.UpsertWebSession(user.GetName(), session)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return session, nil
+}
+
+func (s *AuthServer) createSessionCert(user services.User, sessionTTL time.Duration, publicKey []byte, compatibility string) ([]byte, []byte, error) {
+	roles, err := services.FetchRoles(user.GetRoles(), s.Access, user.GetTraits())
+	if err != nil {
+		return nil, nil, trace.Wrap(err)
+	}
+
+	certs, err := s.generateUserCert(certRequest{
+		user:          user,
+		roles:         roles,
+		ttl:           sessionTTL,
+		publicKey:     publicKey,
+		compatibility: compatibility,
+	})
+	if err != nil {
+		return nil, nil, trace.Wrap(err)
+	}
+
+	return certs.ssh, certs.tls, nil
+}
+
+// createUserParams is a set of parameters used to create a user for an
+// external identity provider.
+type createUserParams struct {
+	// connectorName is the name of the connector for the identity provider.
+	connectorName string
+
+	// username is the Teleport user name .
+	username string
+
+	// logins is the list of *nix logins.
+	logins []string
+
+	// kubeGroups is the list of Kubernetes this user belongs to.
+	kubeGroups []string
+
+	// roles is the list of roles this user is assigned to.
+	roles []string
+
+	// traits is the list of traits for this user.
+	traits map[string][]string
+
+	// sessionTTL is how long this session will last.
+	sessionTTL time.Duration
+}
+
+func (s *AuthServer) calculateGithubUser(connector services.GithubConnector, claims *services.GithubClaims, request *services.GithubAuthRequest) (*createUserParams, error) {
+	p := createUserParams{
+		connectorName: connector.GetName(),
+		username:      claims.Username,
+	}
+
+	// Calculate logins, kubegroups, roles, and traits.
+	p.logins, p.kubeGroups = connector.MapClaims(*claims)
+	if len(p.logins) == 0 {
+		return nil, trace.BadParameter(
 			"user %q does not belong to any teams configured in %q connector",
 			claims.Username, connector.GetName())
 	}
+	p.roles = modules.GetModules().RolesFromLogins(p.logins)
+	p.traits = modules.GetModules().TraitsFromLogins(p.logins, p.kubeGroups)
+
+	// Pick smaller for role: session TTL from role or requested TTL.
+	roles, err := services.FetchRoles(p.roles, s.Access, p.traits)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	roleTTL := roles.AdjustSessionTTL(defaults.MaxCertDuration)
+	p.sessionTTL = utils.MinTTL(roleTTL, request.CertTTL)
+
+	return &p, nil
+}
+
+func (s *AuthServer) createGithubUser(p *createUserParams) (services.User, error) {
+
 	log.WithFields(logrus.Fields{trace.Component: "github"}).Debugf(
 		"Generating dynamic identity %v/%v with logins: %v.",
-		connector.GetName(), claims.Username, logins)
+		p.connectorName, p.username, p.logins)
+
+	expires := s.GetClock().Now().UTC().Add(p.sessionTTL)
+
 	user, err := services.GetUserMarshaler().GenerateUser(&services.UserV2{
 		Kind:    services.KindUser,
 		Version: services.V2,
 		Metadata: services.Metadata{
-			Name:      claims.Username,
+			Name:      p.username,
 			Namespace: defaults.Namespace,
 			Expires:   &expires,
 		},
 		Spec: services.UserSpecV2{
-			Roles:  modules.GetModules().RolesFromLogins(logins),
-			Traits: modules.GetModules().TraitsFromLogins(logins, kubeGroups),
+			Roles:  p.roles,
+			Traits: p.traits,
 			GithubIdentities: []services.ExternalIdentity{{
-				ConnectorID: connector.GetName(),
-				Username:    claims.Username,
+				ConnectorID: p.connectorName,
+				Username:    p.username,
 			}},
 			CreatedBy: services.CreatedBy{
 				User: services.UserRef{Name: "system"},
-				Time: time.Now().UTC(),
+				Time: s.GetClock().Now().UTC(),
 				Connector: &services.ConnectorRef{
 					Type:     teleport.ConnectorGithub,
-					ID:       connector.GetName(),
-					Identity: claims.Username,
+					ID:       p.connectorName,
+					Identity: p.username,
 				},
 			},
 		},
 	})
-	existingUser, err := s.GetUser(claims.Username)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	existingUser, err := s.GetUser(p.username)
 	if err != nil && !trace.IsNotFound(err) {
-		return trace.Wrap(err)
+		return nil, trace.Wrap(err)
 	}
 	if existingUser != nil {
 		ref := user.GetCreatedBy().Connector
 		if !ref.IsSameProvider(existingUser.GetCreatedBy().Connector) {
-			return trace.AlreadyExists("user %q already exists and is not Github user",
+			return nil, trace.AlreadyExists("user %q already exists and is not Github user",
 				existingUser.GetName())
 		}
 	}
 	err = s.UpsertUser(user)
 	if err != nil {
-		return trace.Wrap(err)
+		return nil, trace.Wrap(err)
 	}
-	return nil
+	return user, nil
 }
 
 // populateGithubClaims retrieves information about user and its team

lib/auth/github_test.go

@@ -82,28 +82,14 @@ func (s *GithubSuite) TestPopulateClaims(c *check.C) {
 }
 
 func (s *GithubSuite) TestCreateGithubUser(c *check.C) {
-	connector := services.NewGithubConnector("github", services.GithubConnectorSpecV3{
-		ClientID:     "fakeClientID",
-		ClientSecret: "fakeClientSecret",
-		RedirectURL:  "https://www.example.com",
-		TeamsToLogins: []services.TeamMapping{
-			services.TeamMapping{
-				Organization: "fakeOrg",
-				Team:         "fakeTeam",
-				Logins:       []string{"foo"},
-			},
-		},
-	})
-
-	claims := services.GithubClaims{
-		Username: "foo",
-		OrganizationToTeams: map[string][]string{
-			"fakeOrg": []string{"fakeTeam"},
-		},
-	}
-
 	// Create GitHub user with 1 minute expiry.
-	err := s.a.createGithubUser(connector, claims, s.c.Now().Add(1*time.Minute))
+	_, err := s.a.createGithubUser(&createUserParams{
+		connectorName: "github",
+		username:      "foo",
+		logins:        []string{"foo"},
+		roles:         []string{"admin"},
+		sessionTTL:    1 * time.Minute,
+	})
 	c.Assert(err, check.IsNil)
 
 	// Within that 1 minute period the user should still exist.