Audit log contains login failures.

This commit fixes #1553, fixes #1554 and makes sure
that audit log contains login success and failure entries
for OIDC, SAML, Github and local logins.

lib/auth/auth.go

@@ -339,10 +339,6 @@ func (s *AuthServer) generateUserCert(req certRequest) (*certs, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	s.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
-		events.EventUser:   req.user.GetName(),
-		events.LoginMethod: events.LoginMethodLocal,
-	})
 	return &certs{ssh: sshCert, tls: tlsCert}, nil
 }
 

lib/auth/github.go

@@ -80,7 +80,26 @@ type GithubAuthResponse struct {
 }
 
 // ValidateGithubAuthCallback validates Github auth callback redirect
-func (s *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthResponse, error) {
+func (a *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthResponse, error) {
+	re, err := a.validateGithubAuthCallback(q)
+	if err != nil {
+		a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.LoginMethod:        events.LoginMethodGithub,
+			events.AuthAttemptSuccess: false,
+			events.AuthAttemptErr:     err.Error(),
+		})
+	} else {
+		a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.EventUser:          re.Username,
+			events.AuthAttemptSuccess: true,
+			events.LoginMethod:        events.LoginMethodGithub,
+		})
+	}
+	return re, err
+}
+
+// ValidateGithubAuthCallback validates Github auth callback redirect
+func (s *AuthServer) validateGithubAuthCallback(q url.Values) (*GithubAuthResponse, error) {
 	logger := log.WithFields(logrus.Fields{trace.Component: "github"})
 	error := q.Get("error")
 	if error != "" {
@@ -185,10 +204,6 @@ func (s *AuthServer) ValidateGithubAuthCallback(q url.Values) (*GithubAuthRespon
 			response.HostSigners = append(response.HostSigners, authority)
 		}
 	}
-	s.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
-		events.EventUser:   user.GetName(),
-		events.LoginMethod: events.LoginMethodGithub,
-	})
 	return response, nil
 }
 

lib/auth/methods.go

@@ -24,6 +24,7 @@ import (
 	"golang.org/x/crypto/ssh"
 
 	"github.com/gravitational/teleport"
+	"github.com/gravitational/teleport/lib/events"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
@@ -85,6 +86,25 @@ type SessionCreds struct {
 
 // AuthenticateUser authenticates user based on the request type
 func (s *AuthServer) AuthenticateUser(req AuthenticateUserRequest) error {
+	err := s.authenticateUser(req)
+	if err != nil {
+		s.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.EventUser:          req.Username,
+			events.LoginMethod:        events.LoginMethodLocal,
+			events.AuthAttemptSuccess: false,
+			events.AuthAttemptErr:     err.Error(),
+		})
+	} else {
+		s.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.EventUser:          req.Username,
+			events.LoginMethod:        events.LoginMethodLocal,
+			events.AuthAttemptSuccess: true,
+		})
+	}
+	return err
+}
+
+func (s *AuthServer) authenticateUser(req AuthenticateUserRequest) error {
 	if err := req.CheckAndSetDefaults(); err != nil {
 		return trace.Wrap(err)
 	}

lib/auth/oidc.go

@@ -125,6 +125,24 @@ func (s *AuthServer) CreateOIDCAuthRequest(req services.OIDCAuthRequest) (*servi
 // returned by OIDC Provider, if everything checks out, auth server
 // will respond with OIDCAuthResponse, otherwise it will return error
 func (a *AuthServer) ValidateOIDCAuthCallback(q url.Values) (*OIDCAuthResponse, error) {
+	re, err := a.validateOIDCAuthCallback(q)
+	if err != nil {
+		a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.LoginMethod:        events.LoginMethodOIDC,
+			events.AuthAttemptSuccess: false,
+			events.AuthAttemptErr:     err.Error(),
+		})
+	} else {
+		a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.EventUser:          re.Username,
+			events.AuthAttemptSuccess: true,
+			events.LoginMethod:        events.LoginMethodOIDC,
+		})
+	}
+	return re, err
+}
+
+func (a *AuthServer) validateOIDCAuthCallback(q url.Values) (*OIDCAuthResponse, error) {
 	if error := q.Get("error"); error != "" {
 		return nil, trace.OAuth2(oauth2.ErrorInvalidRequest, error, q)
 	}
@@ -250,10 +268,6 @@ func (a *AuthServer) ValidateOIDCAuthCallback(q url.Values) (*OIDCAuthResponse,
 			response.HostSigners = append(response.HostSigners, authority)
 		}
 	}
-	a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
-		events.EventUser:   user.GetName(),
-		events.LoginMethod: events.LoginMethodOIDC,
-	})
 	return response, nil
 }
 
@@ -378,7 +392,7 @@ func (a *AuthServer) createOIDCUser(connector services.OIDCConnector, ident *oid
 	if existingUser != nil {
 		ref := user.GetCreatedBy().Connector
 		if !ref.IsSameProvider(existingUser.GetCreatedBy().Connector) {
-			return trace.AlreadyExists("user %q already exists and is not OIDC user",
+			return trace.AlreadyExists("user %q already exists and is not OIDC user, remove local user and try again",
 				existingUser.GetName())
 		}
 	}

lib/auth/saml.go

@@ -88,8 +88,8 @@ func (a *AuthServer) buildSAMLRoles(connector services.SAMLConnector, assertionI
 	if len(roles) == 0 {
 		role, err := connector.RoleFromTemplate(assertionInfo)
 		if err != nil {
-			log.Warningf("[SAML] Unable to map claims to roles or role templates for %q: %v", connector.GetName(), err)
-			return nil, trace.AccessDenied("unable to map claims to roles or role templates for %q: %v", connector.GetName(), err)
+			log.Warningf("[SAML] Unable to map claims to roles for %q: %v", connector.GetName(), err)
+			return nil, trace.AccessDenied("unable to map claims to roles for %q: %v", connector.GetName(), err)
 		}
 
 		// figure out ttl for role. expires = now + ttl  =>  ttl = expires - now
@@ -172,7 +172,8 @@ func (a *AuthServer) createSAMLUser(connector services.SAMLConnector, assertionI
 	if existingUser != nil {
 		connectorRef := existingUser.GetCreatedBy().Connector
 		if connectorRef == nil || connectorRef.Type != teleport.ConnectorSAML || connectorRef.ID != connector.GetName() {
-			return trace.AlreadyExists("user %q already exists and is not SAML user", existingUser.GetName())
+			return trace.AlreadyExists("user %q already exists and is not SAML user, remove local user and try again.",
+				existingUser.GetName())
 		}
 	}
 
@@ -246,6 +247,24 @@ type SAMLAuthResponse struct {
 
 // ValidateSAMLResponse consumes attribute statements from SAML identity provider
 func (a *AuthServer) ValidateSAMLResponse(samlResponse string) (*SAMLAuthResponse, error) {
+	re, err := a.validateSAMLResponse(samlResponse)
+	if err != nil {
+		a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.LoginMethod:        events.LoginMethodSAML,
+			events.AuthAttemptSuccess: false,
+			events.AuthAttemptErr:     err.Error(),
+		})
+	} else {
+		a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
+			events.EventUser:          re.Username,
+			events.AuthAttemptSuccess: true,
+			events.LoginMethod:        events.LoginMethodSAML,
+		})
+	}
+	return re, err
+}
+
+func (a *AuthServer) validateSAMLResponse(samlResponse string) (*SAMLAuthResponse, error) {
 	requestID, err := parseSAMLInResponseTo(samlResponse)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -358,9 +377,5 @@ func (a *AuthServer) ValidateSAMLResponse(samlResponse string) (*SAMLAuthRespons
 			response.HostSigners = append(response.HostSigners, authority)
 		}
 	}
-	a.EmitAuditEvent(events.UserLoginEvent, events.EventFields{
-		events.EventUser:   user.GetName(),
-		events.LoginMethod: events.LoginMethodSAML,
-	})
 	return response, nil
 }

lib/services/oidc.go

@@ -464,7 +464,7 @@ func (o *OIDCConnectorV2) RoleFromTemplate(claims jose.Claims) (Role, error) {
 		}
 	}
 
-	return nil, trace.BadParameter("no matching claim name/value, claims: %q", claims)
+	return nil, trace.BadParameter("no matching claim name/value, claims: %v", claims)
 }
 
 // Check returns nil if all parameters are great, err otherwise

lib/services/saml.go

@@ -525,7 +525,7 @@ func (o *SAMLConnectorV2) RoleFromTemplate(assertionInfo saml2.AssertionInfo) (R
 		}
 	}
 
-	return nil, trace.BadParameter("no matching assertion name/value, assertions: %q", assertionMap)
+	return nil, trace.BadParameter("no matching assertion name/value, assertions: %v", assertionMap)
 }
 
 // GetServiceProvider initialises service provider spec from settings

lib/web/apiserver.go

@@ -813,10 +813,11 @@ func (h *Handler) oidcCallback(w http.ResponseWriter, r *http.Request, p httprou
 	if err != nil {
 		log.Warningf("[OIDC] Error while processing callback: %v", err)
 
+		message := "Unable to process callback from OIDC provider. Ask your system administrator to check audit logs for details."
 		// redirect to an error page
 		pathToError := url.URL{
 			Path:     "/web/msg/error/login_failed",
-			RawQuery: url.Values{"details": []string{"Unable to process callback from OIDC provider."}}.Encode(),
+			RawQuery: url.Values{"details": []string{message}}.Encode(),
 		}
 		http.Redirect(w, r, pathToError.String(), http.StatusFound)
 		return nil, nil

lib/web/saml.go

@@ -101,10 +101,12 @@ func (m *Handler) samlACS(w http.ResponseWriter, r *http.Request, p httprouter.P
 	if err != nil {
 		log.Warningf("error while processing callback: %v", err)
 
+		message := "Unable to process callback from SAML provider. Ask your system administrator to check audit logs for details."
+
 		// redirect to an error page
 		pathToError := url.URL{
 			Path:     "/web/msg/error/login_failed",
-			RawQuery: url.Values{"details": []string{"Unable to process callback from SAML provider."}}.Encode(),
+			RawQuery: url.Values{"details": []string{message}}.Encode(),
 		}
 		http.Redirect(w, r, pathToError.String(), http.StatusFound)
 		return nil, nil