diff --git a/constants.go b/constants.go index 4ce9dd2fdd8d2..98d1cc147c9b7 100644 --- a/constants.go +++ b/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" diff --git a/docs/2.3/admin-guide.md b/docs/2.3/admin-guide.md index 79013a48e6964..ad822d41497f0 100644 --- a/docs/2.3/admin-guide.md +++ b/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. diff --git a/lib/auth/github.go b/lib/auth/github.go index ee9b10ca4f2f3..363a280cd2ff9 100644 --- a/lib/auth/github.go +++ b/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", diff --git a/lib/auth/oidc.go b/lib/auth/oidc.go index 03718a3c1fe69..097a28f50c0c3 100644 --- a/lib/auth/oidc.go +++ b/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. diff --git a/lib/services/identity.go b/lib/services/identity.go index 56c8ab5df4add..95f006345125a 100644 --- a/lib/services/identity.go +++ b/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"` diff --git a/lib/services/resource.go b/lib/services/resource.go index c258f07a4d66e..1aa06d4e3cf20 100644 --- a/lib/services/resource.go +++ b/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 diff --git a/lib/services/user.go b/lib/services/user.go index 131c588e9cabb..b505dad214618 100644 --- a/lib/services/user.go +++ b/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 diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 91046fef14cb9..9d07a82cb2264 100644 --- a/lib/web/apiserver.go +++ b/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,10 +739,10 @@ 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) @@ -752,7 +750,7 @@ func (h *Handler) githubCallback(w http.ResponseWriter, r *http.Request, p httpr 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") }