Fix typos and some review comments

constants.go

@@ -116,7 +116,7 @@ const (
 	ConnectorOIDC = "oidc"
 
 	// ConnectorSAML means connector type SAML
-	ConnectorSAML = "oidc"
+	ConnectorSAML = "saml"
 
 	// ConnectorGithub means connector type Github
 	ConnectorGithub = "github"
@@ -152,13 +152,13 @@ const (
 	Local = "local"
 
 	// OIDC means authentication will happen remotely using an OIDC connector.
-	OIDC = "oidc"
+	OIDC = ConnectorOIDC
 
 	// SAML means authentication will happen remotely using a SAML connector.
-	SAML = "saml"
+	SAML = ConnectorSAML
 
 	// Github means authentication will happen remotely using a Github connector.
-	Github = "github"
+	Github = ConnectorGithub
 
 	// JSON means JSON serialization format
 	JSON = "json"

docs/2.3/admin-guide.md

@@ -370,7 +370,7 @@ auth_service:
 
 **Github OAuth 2.0**
 
-This connector implements Github OAuth 2.0 authorization flow. Please refer
+This connector implements Github OAuth 2.0 authentication flow. Please refer
 to Github documentation on [Creating an OAuth App](https://developer.github.com/apps/building-oauth-apps/creating-an-oauth-app/)
 to learn how to create and register an OAuth app.
 
@@ -1050,7 +1050,7 @@ $ tctl create github.yaml
 ```
 
 !!! tip
-    When going through the Github authorizarion flow for the first time,
+    When going through the Github authentication flow for the first time,
     the application must be granted the access to all organizations that
     are present in the "teams to logins" mapping, otherwise Teleport will
     not be able to determine team memberships for these orgs.

lib/auth/github.go

@@ -52,7 +52,7 @@ func (s *AuthServer) CreateGithubAuthRequest(req services.GithubAuthRequest) (*s
 	}
 	req.RedirectURL = client.AuthCodeURL(req.StateToken, "", "")
 	log.WithFields(log.Fields{trace.Component: "github"}).Debugf(
-		"Redirect URL: %v", req.RedirectURL)
+		"Redirect URL: %v.", req.RedirectURL)
 	err = s.Identity.CreateGithubAuthRequest(req, defaults.GithubAuthRequestTTL)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -62,7 +62,7 @@ func (s *AuthServer) CreateGithubAuthRequest(req services.GithubAuthRequest) (*s
 
 // GithubAuthResponse represents Github auth callback validation response
 type GithubAuthResponse struct {
-	// Username is the name of authorized user
+	// Username is the name of authenticated user
 	Username string `json:"username"`
 	// Identity is the external identity
 	Identity services.ExternalIdentity `json:"identity"`
@@ -79,6 +79,7 @@ type GithubAuthResponse struct {
 
 // ValidateGithubAuthCallback validates Github auth callback redirect
 func (s *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthResponse, error) {
+	logger := log.WithFields(log.Fields{trace.Component: "github"})
 	error := q.Get("error")
 	if error != "" {
 		return nil, trace.OAuth2(oauth2.ErrorInvalidRequest, error, q)
@@ -101,29 +102,32 @@ func (s *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthRespon
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	if len(connector.GetTeamsToLogins()) == 0 {
+		logger.Warnf("Github connector %q has empty teams_to_logins mapping, cannot populate claims.",
+			connector.GetName())
+		return nil, trace.BadParameter(
+			"connector %q has empty teams_to_logins mapping", connector.GetName())
+	}
 	client, err := s.getGithubOAuth2Client(connector)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	// exchange the authorization code we got in the callback for access token
+	// exchange the authorization code received by the callback for an access token
 	token, err := client.RequestToken(oauth2.GrantTypeAuthCode, code)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	log.WithFields(log.Fields{trace.Component: "github"}).Debugf(
-		"Obtained OAuth2 token: Type=%v Expires=%v Scope=%v",
+	logger.Debugf("Obtained OAuth2 token: Type=%v Expires=%v Scope=%v.",
 		token.TokenType, token.Expires, token.Scope)
-	// Github does not support OIDC so we have to retrieve user information
-	// by making requests to its API using the access token we just got
+	// Github does not support OIDC so user claims have to be populated
+	// by making requests to Github API using the access token
 	claims, err := populateGithubClaims(&githubAPIClient{token: token.AccessToken})
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	if len(connector.GetTeamsToLogins()) != 0 {
-		err = s.createGithubUser(connector, *claims)
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
+	err = s.createGithubUser(connector, *claims)
+	if err != nil {
+		return nil, trace.Wrap(err)
 	}
 	response := &GithubAuthResponse{
 		Identity: services.ExternalIdentity{
@@ -132,9 +136,6 @@ func (s *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthRespon
 		},
 		Req: *req,
 	}
-	if !req.CheckUser {
-		return response, nil
-	}
 	user, err := s.Identity.GetUserByGithubIdentity(response.Identity)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -196,7 +197,7 @@ func (s *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthRespon
 func (s *AuthServer) createGithubUser(connector services.GithubConnector, claims services.GithubClaims) error {
 	logins := connector.MapClaims(claims)
 	log.WithFields(log.Fields{trace.Component: "github"}).Debugf(
-		"Generating dynamic identity %v/%v with logins: %v",
+		"Generating dynamic identity %v/%v with logins: %v.",
 		connector.GetName(), claims.Email, logins)
 	user, err := services.GetUserMarshaler().GenerateUser(&services.UserV2{
 		Kind:    services.KindUser,
@@ -280,7 +281,7 @@ func populateGithubClaims(client githubAPIClientI) (*services.GithubClaims, erro
 		OrganizationToTeams: orgToTeams,
 	}
 	log.WithFields(log.Fields{trace.Component: "github"}).Debugf(
-		"Claims: %#v", claims)
+		"Claims: %#v.", claims)
 	return claims, nil
 }
 
@@ -415,7 +416,7 @@ const (
 )
 
 var (
-	// GithubScopes is a list of scopes we request during OAuth2 flow
+	// GithubScopes is a list of scopes requested during OAuth2 flow
 	GithubScopes = []string{
 		// user:email grants read-only access to all user's email addresses
 		"user:email",

lib/auth/oidc.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2015 Gravitational, Inc.
+Copyright 2017 Gravitational, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

lib/services/identity.go

@@ -284,8 +284,6 @@ type GithubAuthRequest struct {
 	PublicKey []byte `json:"public_key"`
 	// CertTTL is TTL of the cert that's generated in case of successful auth
 	CertTTL time.Duration `json:"cert_ttl"`
-	// CheckUser tells validator to expect and check the user
-	CheckUser bool `json:"check_user"`
 	// CreateWebSession indicates that a user wants to generate a web session
 	// after successul authentication
 	CreateWebSession bool `json:"create_web_session"`

lib/services/resource.go

@@ -71,7 +71,7 @@ const (
 	// KindOIDCRequest is OIDC auth request resource
 	KindOIDCRequest = "oidc_request"
 
-	// KindOIDCReques is SAML auth request resource
+	// KindSAMLRequest is SAML auth request resource
 	KindSAMLRequest = "saml_request"
 
 	// KindGithubRequest is Github auth request resource

lib/services/user.go

@@ -20,7 +20,7 @@ type User interface {
 	Resource
 	// GetOIDCIdentities returns a list of connected OIDC identities
 	GetOIDCIdentities() []ExternalIdentity
-	// GetSAMLIdentities returns a list of connected OIDC identities
+	// GetSAMLIdentities returns a list of connected SAML identities
 	GetSAMLIdentities() []ExternalIdentity
 	// GetGithubIdentities returns a list of connected Github identities
 	GetGithubIdentities() []ExternalIdentity

lib/web/apiserver.go

@@ -563,7 +563,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou
 	// get all OIDC connectors
 	oidcConnectors, err := h.cfg.ProxyClient.GetOIDCConnectors(false)
 	if err != nil {
-		log.Errorf("Cannot retrieve OIDC connectors: %v", err)
+		log.Errorf("Cannot retrieve OIDC connectors: %v.", err)
 	}
 	for _, item := range oidcConnectors {
 		authProviders = append(authProviders, ui.WebConfigAuthProvider{
@@ -577,7 +577,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou
 	// get all SAML connectors
 	samlConnectors, err := h.cfg.ProxyClient.GetSAMLConnectors(false)
 	if err != nil {
-		log.Errorf("Cannot retrieve SAML connectors: %v", err)
+		log.Errorf("Cannot retrieve SAML connectors: %v.", err)
 	}
 	for _, item := range samlConnectors {
 		authProviders = append(authProviders, ui.WebConfigAuthProvider{
@@ -591,7 +591,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou
 	// get all Github connectors
 	githubConnectors, err := h.cfg.ProxyClient.GetGithubConnectors(false)
 	if err != nil {
-		log.Errorf("Cannot retrieve Github connectors: %v", err)
+		log.Errorf("Cannot retrieve Github connectors: %v.", err)
 	}
 	for _, item := range githubConnectors {
 		authProviders = append(authProviders, ui.WebConfigAuthProvider{
@@ -605,7 +605,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou
 	// get second factor type
 	cap, err := h.cfg.ProxyClient.GetAuthPreference()
 	if err != nil {
-		log.Errorf("Cannot retrieve AuthPreferences: %v", err)
+		log.Errorf("Cannot retrieve AuthPreferences: %v.", err)
 	} else {
 		secondFactor = cap.GetSecondFactor()
 	}
@@ -665,7 +665,7 @@ func (h *Handler) oidcLoginWeb(w http.ResponseWriter, r *http.Request, p httprou
 
 func (h *Handler) githubLoginWeb(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) {
 	logger := log.WithFields(log.Fields{trace.Component: "github"})
-	logger.Debug("Web login start")
+	logger.Debug("Web login start.")
 	clientRedirectURL := r.URL.Query().Get("redirect_url")
 	if clientRedirectURL == "" {
 		return nil, trace.BadParameter("missing redirect_url query parameter")
@@ -676,7 +676,7 @@ func (h *Handler) githubLoginWeb(w http.ResponseWriter, r *http.Request, p httpr
 	}
 	csrfToken, err := csrf.ExtractTokenFromCookie(r)
 	if err != nil {
-		logger.Warnf("Unable to extract CSRF token from cookie: %v", err)
+		logger.Warnf("Unable to extract CSRF token from cookie: %v.", err)
 		return nil, trace.AccessDenied("access denied")
 	}
 	response, err := h.cfg.ProxyClient.CreateGithubAuthRequest(
@@ -685,7 +685,6 @@ func (h *Handler) githubLoginWeb(w http.ResponseWriter, r *http.Request, p httpr
 			ConnectorID:       connectorID,
 			CreateWebSession:  true,
 			ClientRedirectURL: clientRedirectURL,
-			CheckUser:         true,
 		})
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -696,7 +695,7 @@ func (h *Handler) githubLoginWeb(w http.ResponseWriter, r *http.Request, p httpr
 
 func (h *Handler) githubLoginConsole(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) {
 	log.WithFields(log.Fields{trace.Component: "github"}).Debug(
-		"Console login start")
+		"Console login start.")
 	var req client.SSOLoginConsoleReq
 	if err := httplib.ReadJSON(r, &req); err != nil {
 		return nil, trace.Wrap(err)
@@ -709,7 +708,6 @@ func (h *Handler) githubLoginConsole(w http.ResponseWriter, r *http.Request, p h
 			ConnectorID:       req.ConnectorID,
 			PublicKey:         req.PublicKey,
 			CertTTL:           req.CertTTL,
-			CheckUser:         true,
 			ClientRedirectURL: req.RedirectURL,
 			Compatibility:     req.Compatibility,
 		})
@@ -723,16 +721,16 @@ func (h *Handler) githubLoginConsole(w http.ResponseWriter, r *http.Request, p h
 
 func (h *Handler) githubCallback(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) {
 	logger := log.WithFields(log.Fields{trace.Component: "github"})
-	logger.Debugf("Callback start: %v", r.URL.Query())
+	logger.Debugf("Callback start: %v.", r.URL.Query())
 
 	response, err := h.cfg.ProxyClient.ValidateGithubAuthCallback(r.URL.Query())
 	if err != nil {
-		logger.Warnf("Error while processing callback: %v", err)
+		logger.Warnf("Error while processing callback: %v.", err)
 		// redirect to an error page
 		pathToError := url.URL{
 			Path: "/web/msg/error/login_failed",
 			RawQuery: url.Values{"details": []string{
-				"Unable to process callback from Github"}}.Encode(),
+				"Unable to process callback from Github."}}.Encode(),
 		}
 		http.Redirect(w, r, pathToError.String(), http.StatusFound)
 		return nil, nil
@@ -741,18 +739,18 @@ func (h *Handler) githubCallback(w http.ResponseWriter, r *http.Request, p httpr
 	if response.Req.CreateWebSession {
 		err = csrf.VerifyToken(response.Req.CSRFToken, r)
 		if err != nil {
-			logger.Warnf("Unable to verify CSRF token: %v", err)
+			logger.Warnf("Unable to verify CSRF token: %v.", err)
 			return nil, trace.AccessDenied("access denied")
 		}
-		logger.Infof("Callback redirecting to web browser")
+		logger.Infof("Callback is redirecting to web browser.")
 		err = SetSession(w, response.Username, response.Session.GetName())
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
 		httplib.SafeRedirect(w, r, response.Req.ClientRedirectURL)
 		return nil, nil
 	}
-	logger.Infof("Callback redirecting to console login")
+	logger.Infof("Callback is redirecting to console login.")
 	if len(response.Req.PublicKey) == 0 {
 		return nil, trace.BadParameter("not a web or console Github login request")
 	}