diff --git a/lib/auth/clt.go b/lib/auth/clt.go index 728abdd19430d..1877efb58d30c 100644 --- a/lib/auth/clt.go +++ b/lib/auth/clt.go @@ -528,7 +528,7 @@ type IdentityService interface { UpsertOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) // GetOIDCConnector returns OIDC connector information by id GetOIDCConnector(ctx context.Context, id string, withSecrets bool) (types.OIDCConnector, error) - // GetOIDCConnectors gets OIDC connectors list + // GetOIDCConnectors gets valid OIDC connectors list GetOIDCConnectors(ctx context.Context, withSecrets bool) ([]types.OIDCConnector, error) // DeleteOIDCConnector deletes OIDC connector by ID DeleteOIDCConnector(ctx context.Context, connectorID string) error @@ -547,7 +547,7 @@ type IdentityService interface { UpsertSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) // GetSAMLConnector returns SAML connector information by id GetSAMLConnector(ctx context.Context, id string, withSecrets bool) (types.SAMLConnector, error) - // GetSAMLConnectors gets SAML connectors list + // GetSAMLConnectors gets valid SAML connectors list GetSAMLConnectors(ctx context.Context, withSecrets bool) ([]types.SAMLConnector, error) // DeleteSAMLConnector deletes SAML connector by ID DeleteSAMLConnector(ctx context.Context, connectorID string) error @@ -564,7 +564,7 @@ type IdentityService interface { UpdateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) // UpsertGithubConnector creates or updates a Github connector. UpsertGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) - // GetGithubConnectors returns all configured Github connectors + // GetGithubConnectors returns valid Github connectors GetGithubConnectors(ctx context.Context, withSecrets bool) ([]types.GithubConnector, error) // GetGithubConnector returns the specified Github connector GetGithubConnector(ctx context.Context, id string, withSecrets bool) (types.GithubConnector, error) diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index e900a52179783..09bf5511b4431 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -3039,7 +3039,7 @@ func (g *GRPCServer) GetOIDCConnector(ctx context.Context, req *types.ResourceWi return connector, nil } -// GetOIDCConnectors retrieves all OIDC connectors. +// GetOIDCConnectors retrieves valid OIDC connectors, errors from individual connectors are not forwarded. func (g *GRPCServer) GetOIDCConnectors(ctx context.Context, req *types.ResourcesWithSecretsRequest) (*types.OIDCConnectorV3List, error) { auth, err := g.authenticate(ctx) if err != nil { @@ -3183,7 +3183,7 @@ func (g *GRPCServer) GetSAMLConnector(ctx context.Context, req *types.ResourceWi return samlConnectorV2, nil } -// GetSAMLConnectors retrieves all SAML connectors. +// GetSAMLConnectors retrieves valid SAML connectors, errors from individual connectors are not forwarded. func (g *GRPCServer) GetSAMLConnectors(ctx context.Context, req *types.ResourcesWithSecretsRequest) (*types.SAMLConnectorV2List, error) { auth, err := g.authenticate(ctx) if err != nil { @@ -3329,7 +3329,7 @@ func (g *GRPCServer) GetGithubConnector(ctx context.Context, req *types.Resource return githubConnectorV3, nil } -// GetGithubConnectors retrieves all Github connectors. +// GetGithubConnectors retrieves valid GitHub connectors, errors from individual connectors are not forwarded. func (g *GRPCServer) GetGithubConnectors(ctx context.Context, req *types.ResourcesWithSecretsRequest) (*types.GithubConnectorV3List, error) { auth, err := g.authenticate(ctx) if err != nil { diff --git a/lib/services/identity.go b/lib/services/identity.go index f8e7d5a72a709..a93bb27d35d29 100644 --- a/lib/services/identity.go +++ b/lib/services/identity.go @@ -181,7 +181,8 @@ type Identity interface { // GetOIDCConnector returns OIDC connector data, withSecrets adds or removes client secret from return results GetOIDCConnector(ctx context.Context, id string, withSecrets bool) (types.OIDCConnector, error) - // GetOIDCConnectors returns registered connectors, withSecrets adds or removes client secret from return results + // GetOIDCConnectors returns valid registered connectors, withSecrets adds or removes client secret from return + // results. Invalid Connectors are simply logged but errors are not forwarded. GetOIDCConnectors(ctx context.Context, withSecrets bool) ([]types.OIDCConnector, error) // CreateOIDCAuthRequest creates new auth request @@ -203,7 +204,8 @@ type Identity interface { // GetSAMLConnector returns OIDC connector data, withSecrets adds or removes secrets from return results GetSAMLConnector(ctx context.Context, id string, withSecrets bool) (types.SAMLConnector, error) - // GetSAMLConnectors returns registered connectors, withSecrets adds or removes secret from return results + // GetSAMLConnectors returns valid registered connectors, withSecrets adds or removes secret from return results. + // Invalid Connectors are simply logged but errors are not forwarded. GetSAMLConnectors(ctx context.Context, withSecrets bool) ([]types.SAMLConnector, error) // CreateSAMLAuthRequest creates new auth request @@ -225,7 +227,7 @@ type Identity interface { // UpsertGithubConnector creates or updates a Github connector. UpsertGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) - // GetGithubConnectors returns all configured Github connectors + // GetGithubConnectors returns valid Github connectors, invalid Connectors are simply logged but errors are not forwarded. GetGithubConnectors(ctx context.Context, withSecrets bool) ([]types.GithubConnector, error) // GetGithubConnector returns a Github connector by its name diff --git a/lib/services/local/users.go b/lib/services/local/users.go index fd7b8c9d2c686..3e1bb9c61cfd0 100644 --- a/lib/services/local/users.go +++ b/lib/services/local/users.go @@ -1246,17 +1246,21 @@ func (s *IdentityService) GetOIDCConnectors(ctx context.Context, withSecrets boo if err != nil { return nil, trace.Wrap(err) } - connectors := make([]types.OIDCConnector, len(result.Items)) - for i, item := range result.Items { + var connectors []types.OIDCConnector + for _, item := range result.Items { conn, err := services.UnmarshalOIDCConnector(item.Value, services.WithExpires(item.Expires), services.WithRevision(item.Revision)) if err != nil { - return nil, trace.Wrap(err) + logrus. + WithError(err). + WithField("key", item.Key). + Errorf("Error unmarshaling OIDC Connector") + continue } if !withSecrets { conn.SetClientSecret("") conn.SetGoogleServiceAccount("") } - connectors[i] = conn + connectors = append(connectors, conn) } return connectors, nil } @@ -1411,11 +1415,15 @@ func (s *IdentityService) GetSAMLConnectors(ctx context.Context, withSecrets boo if err != nil { return nil, trace.Wrap(err) } - connectors := make([]types.SAMLConnector, len(result.Items)) - for i, item := range result.Items { + var connectors []types.SAMLConnector + for _, item := range result.Items { conn, err := services.UnmarshalSAMLConnector(item.Value, services.WithExpires(item.Expires), services.WithRevision(item.Revision)) if err != nil { - return nil, trace.Wrap(err) + logrus. + WithError(err). + WithField("key", item.Key). + Errorf("Error unmarshaling SAML Connector") + continue } if !withSecrets { keyPair := conn.GetSigningKeyPair() @@ -1424,7 +1432,7 @@ func (s *IdentityService) GetSAMLConnectors(ctx context.Context, withSecrets boo conn.SetSigningKeyPair(keyPair) } } - connectors[i] = conn + connectors = append(connectors, conn) } return connectors, nil } @@ -1600,16 +1608,20 @@ func (s *IdentityService) GetGithubConnectors(ctx context.Context, withSecrets b if err != nil { return nil, trace.Wrap(err) } - connectors := make([]types.GithubConnector, len(result.Items)) - for i, item := range result.Items { + var connectors []types.GithubConnector + for _, item := range result.Items { connector, err := services.UnmarshalGithubConnector(item.Value, services.WithRevision(item.Revision)) if err != nil { - return nil, trace.Wrap(err) + logrus. + WithError(err). + WithField("key", item.Key). + Errorf("Error unmarshaling GitHub Connector") + continue } if !withSecrets { connector.SetClientSecret("") } - connectors[i] = connector + connectors = append(connectors, connector) } return connectors, nil } diff --git a/lib/services/saml.go b/lib/services/saml.go index 7588914a75002..83536fce51463 100644 --- a/lib/services/saml.go +++ b/lib/services/saml.go @@ -41,28 +41,6 @@ import ( "github.com/gravitational/teleport/lib/utils" ) -func setEntityDescriptorFromURL(sc types.SAMLConnector) error { - if sc.GetEntityDescriptorURL() == "" { - return nil - } - - resp, err := http.Get(sc.GetEntityDescriptorURL()) - if err != nil { - return trace.WrapWithMessage(err, "unable to fetch entity descriptor from %v for SAML connector %v", sc.GetEntityDescriptorURL(), sc.GetName()) - } - if resp.StatusCode != http.StatusOK { - return trace.BadParameter("status code %v when fetching from %v for SAML connector %v", resp.StatusCode, sc.GetEntityDescriptorURL(), sc.GetName()) - } - defer resp.Body.Close() - body, err := utils.ReadAtMost(resp.Body, teleport.MaxHTTPResponseSize) - if err != nil { - return trace.Wrap(err) - } - sc.SetEntityDescriptor(string(body)) - log.Debugf("[SAML] Successfully fetched entity descriptor from %v for connector %v", sc.GetEntityDescriptorURL(), sc.GetName()) - return nil -} - // ValidateSAMLConnector validates the SAMLConnector and sets default values. // If a remote to fetch roles is specified, roles will be validated to exist. func ValidateSAMLConnector(sc types.SAMLConnector, rg RoleGetter) error { @@ -70,8 +48,21 @@ func ValidateSAMLConnector(sc types.SAMLConnector, rg RoleGetter) error { return trace.Wrap(err) } - if err := setEntityDescriptorFromURL(sc); err != nil { - log.Errorf("error loading entity descriptor from URL: %s", err) + if sc.GetEntityDescriptorURL() != "" { + resp, err := http.Get(sc.GetEntityDescriptorURL()) + if err != nil { + return trace.WrapWithMessage(err, "unable to fetch entity descriptor from %v for SAML connector %v", sc.GetEntityDescriptorURL(), sc.GetName()) + } + if resp.StatusCode != http.StatusOK { + return trace.BadParameter("status code %v when fetching from %v for SAML connector %v", resp.StatusCode, sc.GetEntityDescriptorURL(), sc.GetName()) + } + defer resp.Body.Close() + body, err := utils.ReadAtMost(resp.Body, teleport.MaxHTTPResponseSize) + if err != nil { + return trace.Wrap(err) + } + sc.SetEntityDescriptor(string(body)) + log.Debugf("[SAML] Successfully fetched entity descriptor from %v for connector %v", sc.GetEntityDescriptorURL(), sc.GetName()) } if sc.GetEntityDescriptor() != "" {