From 8905ec1d1f7b0722149884ade894d73e84bc8e3b Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Fri, 13 Oct 2023 15:32:51 -0400 Subject: [PATCH] Implement Create/Update for auth connectors (#33181) Updates services.UserService and auth.ClientI to contain the Create and Update methods for all auth connectors and implements them at the client, server, and services layer. Upsert was untouched to prevent breaking e and will be addressed in a follow up PR. Contributes to #30416. --- lib/auth/auth_test.go | 44 +++++-- lib/auth/auth_with_roles.go | 94 ++++++++++++++- lib/auth/clt.go | 18 ++- lib/auth/github.go | 77 +++++++++++- lib/auth/grpcserver.go | 126 ++++++++++++++++++++ lib/auth/oidc.go | 46 ++++++- lib/auth/saml.go | 56 +++++++++ lib/services/identity.go | 18 ++- lib/services/local/services_test.go | 1 + lib/services/local/users.go | 159 ++++++++++++++++++++++-- lib/services/suite/suite.go | 179 ++++++++++++++++++++++++++-- 11 files changed, 774 insertions(+), 44 deletions(-) diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index e13c48499b7..938b5858436 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -861,24 +861,33 @@ func TestGithubConnectorCRUDEventsEmitted(t *testing.T) { s := newAuthSuite(t) ctx := context.Background() - // test github create event github, err := types.NewGithubConnector("test", types.GithubConnectorSpecV3{ - TeamsToLogins: []types.TeamMapping{ + TeamsToRoles: []types.TeamRolesMapping{ { Organization: "octocats", Team: "dummy", - Logins: []string{"dummy"}, + Roles: []string{"dummy"}, }, }, }) + // test github create event require.NoError(t, err) - err = s.a.upsertGithubConnector(ctx, github) + github, err = s.a.createGithubConnector(ctx, github) require.NoError(t, err) require.IsType(t, &apievents.GithubConnectorCreate{}, s.mockEmitter.LastEvent()) require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.GithubConnectorCreatedEvent) s.mockEmitter.Reset() // test github update event + github.SetDisplay("llama") + github, err = s.a.updateGithubConnector(ctx, github) + require.NoError(t, err) + require.IsType(t, &apievents.GithubConnectorCreate{}, s.mockEmitter.LastEvent()) + require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.GithubConnectorCreatedEvent) + s.mockEmitter.Reset() + + // test github upsert event + github.SetDisplay("alpaca") err = s.a.upsertGithubConnector(ctx, github) require.NoError(t, err) require.IsType(t, &apievents.GithubConnectorCreate{}, s.mockEmitter.LastEvent()) @@ -897,7 +906,6 @@ func TestOIDCConnectorCRUDEventsEmitted(t *testing.T) { s := newAuthSuite(t) ctx := context.Background() - // test oidc create event oidc, err := types.NewOIDCConnector("test", types.OIDCConnectorSpecV3{ ClientID: "a", ClaimsToRoles: []types.ClaimMapping{ @@ -910,13 +918,24 @@ func TestOIDCConnectorCRUDEventsEmitted(t *testing.T) { RedirectURLs: []string{"https://proxy.example.com/v1/webapi/oidc/callback"}, }) require.NoError(t, err) - err = s.a.UpsertOIDCConnector(ctx, oidc) + + // test oidc create event + oidc, err = s.a.CreateOIDCConnector(ctx, oidc) require.NoError(t, err) require.IsType(t, &apievents.OIDCConnectorCreate{}, s.mockEmitter.LastEvent()) require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.OIDCConnectorCreatedEvent) s.mockEmitter.Reset() // test oidc update event + oidc.SetDisplay("llama") + oidc, err = s.a.UpdateOIDCConnector(ctx, oidc) + require.NoError(t, err) + require.IsType(t, &apievents.OIDCConnectorCreate{}, s.mockEmitter.LastEvent()) + require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.OIDCConnectorCreatedEvent) + s.mockEmitter.Reset() + + // test oidc upsert event + oidc.SetDisplay("alpaca") err = s.a.UpsertOIDCConnector(ctx, oidc) require.NoError(t, err) require.IsType(t, &apievents.OIDCConnectorCreate{}, s.mockEmitter.LastEvent()) @@ -957,7 +976,6 @@ func TestSAMLConnectorCRUDEventsEmitted(t *testing.T) { err = s.a.CreateRole(ctx, role) require.NoError(t, err) - // test saml create saml, err := types.NewSAMLConnector("test", types.SAMLConnectorSpecV2{ AssertionConsumerService: "a", Issuer: "b", @@ -973,13 +991,23 @@ func TestSAMLConnectorCRUDEventsEmitted(t *testing.T) { }) require.NoError(t, err) - err = s.a.UpsertSAMLConnector(ctx, saml) + // test saml create + saml, err = s.a.CreateSAMLConnector(ctx, saml) require.NoError(t, err) require.IsType(t, &apievents.SAMLConnectorCreate{}, s.mockEmitter.LastEvent()) require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.SAMLConnectorCreatedEvent) s.mockEmitter.Reset() // test saml update event + saml.SetDisplay("llama") + saml, err = s.a.UpdateSAMLConnector(ctx, saml) + require.NoError(t, err) + require.IsType(t, &apievents.SAMLConnectorCreate{}, s.mockEmitter.LastEvent()) + require.Equal(t, s.mockEmitter.LastEvent().GetType(), events.SAMLConnectorCreatedEvent) + s.mockEmitter.Reset() + + // test saml upsert event + saml.SetDisplay("alapaca") err = s.a.UpsertSAMLConnector(ctx, saml) require.NoError(t, err) require.IsType(t, &apievents.SAMLConnectorCreate{}, s.mockEmitter.LastEvent()) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 3d4677fe035..9d9a4eb55c7 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -3421,7 +3421,40 @@ func (a *ServerWithRoles) UpsertOIDCConnector(ctx context.Context, connector typ return trace.AccessDenied("OIDC is only available in Teleport Enterprise") } - return a.authServer.UpsertOIDCConnector(ctx, connector) + err := a.authServer.UpsertOIDCConnector(ctx, connector) + return trace.Wrap(err) +} + +// UpdateOIDCConnector updates an existing OIDC connector. +func (a *ServerWithRoles) UpdateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) { + if err := a.authConnectorAction(apidefaults.Namespace, types.KindOIDC, types.VerbUpdate); err != nil { + return nil, trace.Wrap(err) + } + if !modules.GetModules().Features().OIDC { + // TODO(zmb3): ideally we would wrap ErrRequiresEnterprise here, but + // we can't currently propagate wrapped errors across the gRPC boundary, + // and we want tctl to display a clean user-facing message in this case + return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") + } + + updated, err := a.authServer.UpdateOIDCConnector(ctx, connector) + return updated, trace.Wrap(err) +} + +// CreateOIDCConnector creates a new OIDC connector. +func (a *ServerWithRoles) CreateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) { + if err := a.authConnectorAction(apidefaults.Namespace, types.KindOIDC, types.VerbCreate); err != nil { + return nil, trace.Wrap(err) + } + if !modules.GetModules().Features().OIDC { + // TODO(zmb3): ideally we would wrap ErrRequiresEnterprise here, but + // we can't currently propagate wrapped errors across the gRPC boundary, + // and we want tctl to display a clean user-facing message in this case + return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") + } + + creted, err := a.authServer.CreateOIDCConnector(ctx, connector) + return creted, trace.Wrap(err) } func (a *ServerWithRoles) GetOIDCConnector(ctx context.Context, id string, withSecrets bool) (types.OIDCConnector, error) { @@ -3506,7 +3539,36 @@ func (a *ServerWithRoles) UpsertSAMLConnector(ctx context.Context, connector typ return trace.Wrap(err) } - return a.authServer.UpsertSAMLConnector(ctx, connector) + err := a.authServer.UpsertSAMLConnector(ctx, connector) + return trace.Wrap(err) +} + +// CreateSAMLConnector creates a new SAML connector. +func (a *ServerWithRoles) CreateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) { + if !modules.GetModules().Features().SAML { + return nil, trace.Wrap(ErrSAMLRequiresEnterprise) + } + + if err := a.authConnectorAction(apidefaults.Namespace, types.KindSAML, types.VerbCreate); err != nil { + return nil, trace.Wrap(err) + } + + created, err := a.authServer.CreateSAMLConnector(ctx, connector) + return created, trace.Wrap(err) +} + +// UpdateSAMLConnector updates an existing SAML connector +func (a *ServerWithRoles) UpdateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) { + if !modules.GetModules().Features().SAML { + return nil, trace.Wrap(ErrSAMLRequiresEnterprise) + } + + if err := a.authConnectorAction(apidefaults.Namespace, types.KindSAML, types.VerbUpdate); err != nil { + return nil, trace.Wrap(err) + } + + updated, err := a.authServer.UpdateSAMLConnector(ctx, connector) + return updated, trace.Wrap(err) } func (a *ServerWithRoles) GetSAMLConnector(ctx context.Context, id string, withSecrets bool) (types.SAMLConnector, error) { @@ -3632,7 +3694,33 @@ func (a *ServerWithRoles) UpsertGithubConnector(ctx context.Context, connector t if err := a.checkGithubConnector(connector); err != nil { return trace.Wrap(err) } - return a.authServer.upsertGithubConnector(ctx, connector) + + err := a.authServer.upsertGithubConnector(ctx, connector) + return trace.Wrap(err) +} + +// CreateGithubConnector creates a new Github connector. +func (a *ServerWithRoles) CreateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + if err := a.authConnectorAction(apidefaults.Namespace, types.KindGithub, types.VerbCreate); err != nil { + return nil, trace.Wrap(err) + } + if err := a.checkGithubConnector(connector); err != nil { + return nil, trace.Wrap(err) + } + created, err := a.authServer.createGithubConnector(ctx, connector) + return created, trace.Wrap(err) +} + +// UpdateGithubConnector updates an existing Github connector. +func (a *ServerWithRoles) UpdateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + if err := a.authConnectorAction(apidefaults.Namespace, types.KindGithub, types.VerbUpdate); err != nil { + return nil, trace.Wrap(err) + } + if err := a.checkGithubConnector(connector); err != nil { + return nil, trace.Wrap(err) + } + updated, err := a.authServer.updateGithubConnector(ctx, connector) + return updated, trace.Wrap(err) } func (a *ServerWithRoles) GetGithubConnector(ctx context.Context, id string, withSecrets bool) (types.GithubConnector, error) { diff --git a/lib/auth/clt.go b/lib/auth/clt.go index 46b9e0e4fb8..1938ddc354a 100644 --- a/lib/auth/clt.go +++ b/lib/auth/clt.go @@ -494,7 +494,11 @@ type WebService interface { // IdentityService manages identities and users type IdentityService interface { - // UpsertOIDCConnector updates or creates OIDC connector + // CreateOIDCConnector creates a new OIDC connector. + CreateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) + // UpdateOIDCConnector updates an existing OIDC connector. + UpdateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) + // UpsertOIDCConnector updates or creates an OIDC connector. UpsertOIDCConnector(ctx context.Context, connector types.OIDCConnector) error // GetOIDCConnector returns OIDC connector information by id GetOIDCConnector(ctx context.Context, id string, withSecrets bool) (types.OIDCConnector, error) @@ -509,7 +513,11 @@ type IdentityService interface { // ValidateOIDCAuthCallback validates OIDC auth callback returned from redirect ValidateOIDCAuthCallback(ctx context.Context, q url.Values) (*OIDCAuthResponse, error) - // UpsertSAMLConnector updates or creates SAML connector + // CreateSAMLConnector creates a new SAML connector. + CreateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) + // UpdateSAMLConnector updates an existing SAML connector + UpdateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) + // UpsertSAMLConnector updates or creates a SAML connector UpsertSAMLConnector(ctx context.Context, connector types.SAMLConnector) error // GetSAMLConnector returns SAML connector information by id GetSAMLConnector(ctx context.Context, id string, withSecrets bool) (types.SAMLConnector, error) @@ -524,7 +532,11 @@ type IdentityService interface { // GetSAMLAuthRequest returns SAML auth request if found GetSAMLAuthRequest(ctx context.Context, authRequestID string) (*types.SAMLAuthRequest, error) - // UpsertGithubConnector creates or updates a Github connector + // CreateGithubConnector creates a new Github connector. + CreateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) + // UpdateGithubConnector updates an existing Github connector. + UpdateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) + // UpsertGithubConnector creates or updates a Github connector. UpsertGithubConnector(ctx context.Context, connector types.GithubConnector) error // GetGithubConnectors returns all configured Github connectors GetGithubConnectors(ctx context.Context, withSecrets bool) ([]types.GithubConnector, error) diff --git a/lib/auth/github.go b/lib/auth/github.go index 330bc8eff4a..1a5a11d53ab 100644 --- a/lib/auth/github.go +++ b/lib/auth/github.go @@ -103,7 +103,29 @@ func (g *GithubConverter) UpsertGithubConnector(ctx context.Context, connector t if err != nil { return trace.Wrap(err) } - return g.ClientI.UpsertGithubConnector(ctx, convertedConnector) + + err = g.ClientI.UpsertGithubConnector(ctx, convertedConnector) + return trace.Wrap(err) +} + +func (g *GithubConverter) CreateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + convertedConnector, err := services.ConvertGithubConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + + created, err := g.ClientI.CreateGithubConnector(ctx, convertedConnector) + return created, trace.Wrap(err) +} + +func (g *GithubConverter) UpdateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + convertedConnector, err := services.ConvertGithubConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + + updated, err := g.ClientI.UpdateGithubConnector(ctx, convertedConnector) + return updated, trace.Wrap(err) } // CreateGithubAuthRequest creates a new request for Github OAuth2 flow @@ -135,7 +157,7 @@ func (a *Server) upsertGithubConnector(ctx context.Context, connector types.Gith if err := a.UpsertGithubConnector(ctx, connector); err != nil { return trace.Wrap(err) } - if err := a.emitter.EmitAuditEvent(a.closeCtx, &apievents.GithubConnectorCreate{ + if err := a.emitter.EmitAuditEvent(ctx, &apievents.GithubConnectorCreate{ Metadata: apievents.Metadata{ Type: events.GithubConnectorCreatedEvent, Code: events.GithubConnectorCreatedCode, @@ -151,6 +173,57 @@ func (a *Server) upsertGithubConnector(ctx context.Context, connector types.Gith return nil } +// createGithubConnector creates a new Github connector. +func (a *Server) createGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + if err := checkGithubOrgSSOSupport(ctx, connector, nil, a.githubOrgSSOCache, nil); err != nil { + return nil, trace.Wrap(err) + } + + created, err := a.CreateGithubConnector(ctx, connector) + if err != nil { + return nil, trace.Wrap(err) + } + if err := a.emitter.EmitAuditEvent(ctx, &apievents.GithubConnectorCreate{ + Metadata: apievents.Metadata{ + Type: events.GithubConnectorCreatedEvent, + Code: events.GithubConnectorCreatedCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: connector.GetName(), + }, + }); err != nil { + log.WithError(err).Warn("Failed to emit GitHub connector create event.") + } + + return created, nil +} + +// updateGithubConnector updates an existing Github connector. +func (a *Server) updateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + if err := checkGithubOrgSSOSupport(ctx, connector, nil, a.githubOrgSSOCache, nil); err != nil { + return nil, trace.Wrap(err) + } + updated, err := a.UpdateGithubConnector(ctx, connector) + if err != nil { + return nil, trace.Wrap(err) + } + if err := a.emitter.EmitAuditEvent(ctx, &apievents.GithubConnectorCreate{ + Metadata: apievents.Metadata{ + Type: events.GithubConnectorCreatedEvent, + Code: events.GithubConnectorCreatedCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: connector.GetName(), + }, + }); err != nil { + log.WithError(err).Warn("Failed to emit GitHub connector create event.") + } + + return updated, nil +} + // httpRequester allows a net/http.Client to be mocked for tests. // TODO(capnspacehook): test without using this interface type httpRequester interface { diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index 463680abe80..9655e1b8e3d 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -2950,6 +2950,46 @@ func (g *GRPCServer) GetOIDCConnectors(ctx context.Context, req *types.Resources }, nil } +// CreateOIDCConnector creates a new OIDC connector. +func (g *GRPCServer) CreateOIDCConnector(ctx context.Context, req *authpb.CreateOIDCConnectorRequest) (*types.OIDCConnectorV3, error) { + auth, err := g.authenticate(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + created, err := auth.ServerWithRoles.CreateOIDCConnector(ctx, req.Connector) + if err != nil { + return nil, trace.Wrap(err) + } + + v3, ok := created.(*types.OIDCConnectorV3) + if !ok { + return nil, trace.Errorf("encountered unexpected OIDC connector type: %T", created) + } + + return v3, nil +} + +// UpdateOIDCConnector updates an existing OIDC connector. +func (g *GRPCServer) UpdateOIDCConnector(ctx context.Context, req *authpb.UpdateOIDCConnectorRequest) (*types.OIDCConnectorV3, error) { + auth, err := g.authenticate(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + updated, err := auth.ServerWithRoles.UpdateOIDCConnector(ctx, req.Connector) + if err != nil { + return nil, trace.Wrap(err) + } + + v3, ok := updated.(*types.OIDCConnectorV3) + if !ok { + return nil, trace.Errorf("encountered unexpected OIDC connector type: %T", updated) + } + + return v3, nil +} + // UpsertOIDCConnector upserts an OIDC connector. func (g *GRPCServer) UpsertOIDCConnector(ctx context.Context, oidcConnector *types.OIDCConnectorV3) (*emptypb.Empty, error) { auth, err := g.authenticate(ctx) @@ -3039,6 +3079,46 @@ func (g *GRPCServer) GetSAMLConnectors(ctx context.Context, req *types.Resources }, nil } +// CreateSAMLConnector creates a new SAML connector. +func (g *GRPCServer) CreateSAMLConnector(ctx context.Context, req *authpb.CreateSAMLConnectorRequest) (*types.SAMLConnectorV2, error) { + auth, err := g.authenticate(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + created, err := auth.ServerWithRoles.CreateSAMLConnector(ctx, req.Connector) + if err != nil { + return nil, trace.Wrap(err) + } + + v2, ok := created.(*types.SAMLConnectorV2) + if !ok { + return nil, trace.Errorf("encountered unexpected SAML connector type: %T", created) + } + + return v2, nil +} + +// UpdateSAMLConnector updates an existing SAML connector. +func (g *GRPCServer) UpdateSAMLConnector(ctx context.Context, req *authpb.UpdateSAMLConnectorRequest) (*types.SAMLConnectorV2, error) { + auth, err := g.authenticate(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + + updated, err := auth.ServerWithRoles.UpdateSAMLConnector(ctx, req.Connector) + if err != nil { + return nil, trace.Wrap(err) + } + + v2, ok := updated.(*types.SAMLConnectorV2) + if !ok { + return nil, trace.Errorf("encountered unexpected SAML connector type: %T", updated) + } + + return v2, nil +} + // UpsertSAMLConnector upserts a SAML connector. func (g *GRPCServer) UpsertSAMLConnector(ctx context.Context, samlConnector *types.SAMLConnectorV2) (*emptypb.Empty, error) { auth, err := g.authenticate(ctx) @@ -3144,6 +3224,52 @@ func (g *GRPCServer) UpsertGithubConnector(ctx context.Context, connector *types return &emptypb.Empty{}, nil } +// UpdateGithubConnector updates an existing Github connector. +func (g *GRPCServer) UpdateGithubConnector(ctx context.Context, req *authpb.UpdateGithubConnectorRequest) (*types.GithubConnectorV3, error) { + auth, err := g.authenticate(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + githubConnector, err := services.InitGithubConnector(req.Connector) + if err != nil { + return nil, trace.Wrap(err) + } + + updated, err := auth.ServerWithRoles.UpdateGithubConnector(ctx, githubConnector) + if err != nil { + return nil, trace.Wrap(err) + } + + githubConnectorV3, ok := updated.(*types.GithubConnectorV3) + if !ok { + return nil, trace.Errorf("encountered unexpected GitHub connector type: %T", updated) + } + return githubConnectorV3, nil +} + +// CreateGithubConnector creates a new Github connector. +func (g *GRPCServer) CreateGithubConnector(ctx context.Context, req *authpb.CreateGithubConnectorRequest) (*types.GithubConnectorV3, error) { + auth, err := g.authenticate(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + githubConnector, err := services.InitGithubConnector(req.Connector) + if err != nil { + return nil, trace.Wrap(err) + } + + created, err := auth.ServerWithRoles.CreateGithubConnector(ctx, githubConnector) + if err != nil { + return nil, trace.Wrap(err) + } + + githubConnectorV3, ok := created.(*types.GithubConnectorV3) + if !ok { + return nil, trace.Errorf("encountered unexpected GitHub connector type: %T", created) + } + return githubConnectorV3, nil +} + // DeleteGithubConnector deletes a Github connector by name. func (g *GRPCServer) DeleteGithubConnector(ctx context.Context, req *types.ResourceRequest) (*emptypb.Empty, error) { auth, err := g.authenticate(ctx) diff --git a/lib/auth/oidc.go b/lib/auth/oidc.go index 335d1923cf1..06e00044607 100644 --- a/lib/auth/oidc.go +++ b/lib/auth/oidc.go @@ -34,7 +34,7 @@ type OIDCService interface { ValidateOIDCAuthCallback(ctx context.Context, q url.Values) (*OIDCAuthResponse, error) } -var errOIDCNotImplemented = trace.AccessDenied("OIDC is only available in enterprise subscriptions") +var errOIDCNotImplemented = &trace.AccessDeniedError{Message: "OIDC is only available in enterprise subscriptions"} // UpsertOIDCConnector creates or updates an OIDC connector. func (a *Server) UpsertOIDCConnector(ctx context.Context, connector types.OIDCConnector) error { @@ -57,6 +57,50 @@ func (a *Server) UpsertOIDCConnector(ctx context.Context, connector types.OIDCCo return nil } +// UpdateOIDCConnector updates an existing OIDC connector. +func (a *Server) UpdateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) { + updated, err := a.Services.UpdateOIDCConnector(ctx, connector) + if err != nil { + return nil, trace.Wrap(err) + } + if err := a.emitter.EmitAuditEvent(ctx, &apievents.OIDCConnectorCreate{ + Metadata: apievents.Metadata{ + Type: events.OIDCConnectorCreatedEvent, + Code: events.OIDCConnectorCreatedCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: connector.GetName(), + }, + }); err != nil { + log.WithError(err).Warn("Failed to emit OIDC connector create event.") + } + + return updated, nil +} + +// CreateOIDCConnector creates a new OIDC connector. +func (a *Server) CreateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) { + created, err := a.Services.CreateOIDCConnector(ctx, connector) + if err != nil { + return nil, trace.Wrap(err) + } + if err := a.emitter.EmitAuditEvent(ctx, &apievents.OIDCConnectorCreate{ + Metadata: apievents.Metadata{ + Type: events.OIDCConnectorCreatedEvent, + Code: events.OIDCConnectorCreatedCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: connector.GetName(), + }, + }); err != nil { + log.WithError(err).Warn("Failed to emit OIDC connector create event.") + } + + return created, nil +} + // DeleteOIDCConnector deletes an OIDC connector by name. func (a *Server) DeleteOIDCConnector(ctx context.Context, connectorName string) error { if err := a.Services.DeleteOIDCConnector(ctx, connectorName); err != nil { diff --git a/lib/auth/saml.go b/lib/auth/saml.go index c629414b779..d05671e68fa 100644 --- a/lib/auth/saml.go +++ b/lib/auth/saml.go @@ -75,6 +75,62 @@ func (a *Server) UpsertSAMLConnector(ctx context.Context, connector types.SAMLCo return nil } +// UpdateSAMLConnector updates an existing SAML connector. +func (a *Server) UpdateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) { + // Validate the SAML connector here, because even though Services.UpsertSAMLConnector + // also validates, it does not have a RoleGetter to use to validate the roles, so + // has to pass `nil` for the second argument. + if err := services.ValidateSAMLConnector(connector, a); err != nil { + return nil, trace.Wrap(err) + } + updated, err := a.Services.UpdateSAMLConnector(ctx, connector) + if err != nil { + return nil, trace.Wrap(err) + } + if err := a.emitter.EmitAuditEvent(ctx, &apievents.SAMLConnectorCreate{ + Metadata: apievents.Metadata{ + Type: events.SAMLConnectorCreatedEvent, + Code: events.SAMLConnectorCreatedCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: connector.GetName(), + }, + }); err != nil { + log.WithError(err).Warn("Failed to emit SAML connector create event.") + } + + return updated, nil +} + +// CreateSAMLConnector creates a new SAML connector. +func (a *Server) CreateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) { + // Validate the SAML connector here, because even though Services.UpsertSAMLConnector + // also validates, it does not have a RoleGetter to use to validate the roles, so + // has to pass `nil` for the second argument. + if err := services.ValidateSAMLConnector(connector, a); err != nil { + return nil, trace.Wrap(err) + } + created, err := a.Services.CreateSAMLConnector(ctx, connector) + if err != nil { + return nil, trace.Wrap(err) + } + if err := a.emitter.EmitAuditEvent(ctx, &apievents.SAMLConnectorCreate{ + Metadata: apievents.Metadata{ + Type: events.SAMLConnectorCreatedEvent, + Code: events.SAMLConnectorCreatedCode, + }, + UserMetadata: authz.ClientUserMetadata(ctx), + ResourceMetadata: apievents.ResourceMetadata{ + Name: connector.GetName(), + }, + }); err != nil { + log.WithError(err).Warn("Failed to emit SAML connector create event.") + } + + return created, nil +} + // DeleteSAMLConnector deletes a SAML connector. func (a *Server) DeleteSAMLConnector(ctx context.Context, connectorID string) error { if err := a.Services.DeleteSAMLConnector(ctx, connectorID); err != nil { diff --git a/lib/services/identity.go b/lib/services/identity.go index fa10c0d00fd..e5f2f8a3fbe 100644 --- a/lib/services/identity.go +++ b/lib/services/identity.go @@ -163,7 +163,11 @@ type Identity interface { // DeleteMFADevice deletes an MFA device for the user by ID. DeleteMFADevice(ctx context.Context, user, id string) error - // UpsertOIDCConnector upserts OIDC Connector + // CreateOIDCConnector creates a new OIDC connector. + CreateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) + // UpdateOIDCConnector updates an existing OIDC connector. + UpdateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) + // UpsertOIDCConnector updates or creates an OIDC connector. UpsertOIDCConnector(ctx context.Context, connector types.OIDCConnector) error // DeleteOIDCConnector deletes OIDC Connector @@ -181,7 +185,11 @@ type Identity interface { // GetOIDCAuthRequest returns OIDC auth request if found GetOIDCAuthRequest(ctx context.Context, stateToken string) (*types.OIDCAuthRequest, error) - // UpsertSAMLConnector upserts SAML Connector + // CreateSAMLConnector creates a new SAML connector. + CreateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) + // UpdateSAMLConnector updates an existing SAML connector + UpdateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) + // UpsertSAMLConnector updates or creates a SAML connector UpsertSAMLConnector(ctx context.Context, connector types.SAMLConnector) error // DeleteSAMLConnector deletes OIDC Connector @@ -205,7 +213,11 @@ type Identity interface { // GetSSODiagnosticInfo returns SSO diagnostic info records. GetSSODiagnosticInfo(ctx context.Context, authKind string, authRequestID string) (*types.SSODiagnosticInfo, error) - // UpsertGithubConnector creates or updates a new Github connector + // CreateGithubConnector creates a new Github connector. + CreateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) + // UpdateGithubConnector updates an existing Github connector. + UpdateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) + // UpsertGithubConnector creates or updates a Github connector. UpsertGithubConnector(ctx context.Context, connector types.GithubConnector) error // GetGithubConnectors returns all configured Github connectors diff --git a/lib/services/local/services_test.go b/lib/services/local/services_test.go index cdd9bb6860e..923b5d891f2 100644 --- a/lib/services/local/services_test.go +++ b/lib/services/local/services_test.go @@ -99,6 +99,7 @@ func TestCRUD(t *testing.T) { t.Run("TestEvents", tt.suite.Events) t.Run("TestEventsClusterConfig", tt.suite.EventsClusterConfig) t.Run("TestNetworkRestrictions", func(t *testing.T) { tt.suite.NetworkRestrictions(t) }) + t.Run("TestOIDCCRUD", tt.suite.OIDCCRUD) } func TestSemaphore(t *testing.T) { diff --git a/lib/services/local/users.go b/lib/services/local/users.go index 09269c7fefc..42148c29afe 100644 --- a/lib/services/local/users.go +++ b/lib/services/local/users.go @@ -980,13 +980,55 @@ func (s *IdentityService) UpsertOIDCConnector(ctx context.Context, connector typ ID: connector.GetResourceID(), Revision: rev, } - _, err = s.Put(ctx, item) + lease, err := s.Put(ctx, item) if err != nil { return trace.Wrap(err) } + connector.SetRevision(lease.Revision) return nil } +// CreateOIDCConnector creates a new OIDC connector. +func (s *IdentityService) CreateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) { + value, err := services.MarshalOIDCConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + item := backend.Item{ + Key: backend.Key(webPrefix, connectorsPrefix, oidcPrefix, connectorsPrefix, connector.GetName()), + Value: value, + Expires: connector.Expiry(), + ID: connector.GetResourceID(), + } + lease, err := s.Create(ctx, item) + if err != nil { + return nil, trace.Wrap(err) + } + connector.SetRevision(lease.Revision) + return connector, nil +} + +// UpdateOIDCConnector updates an existing OIDC connector. +func (s *IdentityService) UpdateOIDCConnector(ctx context.Context, connector types.OIDCConnector) (types.OIDCConnector, error) { + value, err := services.MarshalOIDCConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + item := backend.Item{ + Key: backend.Key(webPrefix, connectorsPrefix, oidcPrefix, connectorsPrefix, connector.GetName()), + Value: value, + Expires: connector.Expiry(), + ID: connector.GetResourceID(), + Revision: connector.GetRevision(), + } + lease, err := s.ConditionalUpdate(ctx, item) + if err != nil { + return nil, trace.Wrap(err) + } + connector.SetRevision(lease.Revision) + return connector, nil +} + // DeleteOIDCConnector deletes OIDC Connector by name func (s *IdentityService) DeleteOIDCConnector(ctx context.Context, name string) error { if name == "" { @@ -1009,8 +1051,7 @@ func (s *IdentityService) GetOIDCConnector(ctx context.Context, name string, wit } return nil, trace.Wrap(err) } - conn, err := services.UnmarshalOIDCConnector(item.Value, - services.WithExpires(item.Expires)) + conn, err := services.UnmarshalOIDCConnector(item.Value, services.WithExpires(item.Expires), services.WithRevision(item.Revision)) if err != nil { return nil, trace.Wrap(err) } @@ -1030,8 +1071,7 @@ func (s *IdentityService) GetOIDCConnectors(ctx context.Context, withSecrets boo } connectors := make([]types.OIDCConnector, len(result.Items)) for i, item := range result.Items { - conn, err := services.UnmarshalOIDCConnector( - item.Value, services.WithExpires(item.Expires)) + conn, err := services.UnmarshalOIDCConnector(item.Value, services.WithExpires(item.Expires), services.WithRevision(item.Revision)) if err != nil { return nil, trace.Wrap(err) } @@ -1096,13 +1136,60 @@ func (s *IdentityService) UpsertSAMLConnector(ctx context.Context, connector typ Expires: connector.Expiry(), Revision: rev, } - _, err = s.Put(ctx, item) + lease, err := s.Put(ctx, item) if err != nil { return trace.Wrap(err) } + connector.SetRevision(lease.Revision) return nil } +// UpdateSAMLConnector updates an existing SAML connector +func (s *IdentityService) UpdateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) { + if err := services.ValidateSAMLConnector(connector, nil); err != nil { + return nil, trace.Wrap(err) + } + value, err := services.MarshalSAMLConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + item := backend.Item{ + Key: backend.Key(webPrefix, connectorsPrefix, samlPrefix, connectorsPrefix, connector.GetName()), + Value: value, + Expires: connector.Expiry(), + ID: connector.GetResourceID(), + Revision: connector.GetRevision(), + } + lease, err := s.ConditionalUpdate(ctx, item) + if err != nil { + return nil, trace.Wrap(err) + } + connector.SetRevision(lease.Revision) + return connector, nil +} + +// CreateSAMLConnector creates a new SAML connector. +func (s *IdentityService) CreateSAMLConnector(ctx context.Context, connector types.SAMLConnector) (types.SAMLConnector, error) { + if err := services.ValidateSAMLConnector(connector, nil); err != nil { + return nil, trace.Wrap(err) + } + value, err := services.MarshalSAMLConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + item := backend.Item{ + Key: backend.Key(webPrefix, connectorsPrefix, samlPrefix, connectorsPrefix, connector.GetName()), + Value: value, + Expires: connector.Expiry(), + } + lease, err := s.Create(ctx, item) + if err != nil { + return nil, trace.Wrap(err) + } + connector.SetRevision(lease.Revision) + return connector, nil +} + // DeleteSAMLConnector deletes SAML Connector by name func (s *IdentityService) DeleteSAMLConnector(ctx context.Context, name string) error { if name == "" { @@ -1125,8 +1212,7 @@ func (s *IdentityService) GetSAMLConnector(ctx context.Context, name string, wit } return nil, trace.Wrap(err) } - conn, err := services.UnmarshalSAMLConnector( - item.Value, services.WithExpires(item.Expires)) + conn, err := services.UnmarshalSAMLConnector(item.Value, services.WithExpires(item.Expires), services.WithRevision(item.Revision)) if err != nil { return nil, trace.Wrap(err) } @@ -1150,8 +1236,7 @@ func (s *IdentityService) GetSAMLConnectors(ctx context.Context, withSecrets boo } connectors := make([]types.SAMLConnector, len(result.Items)) for i, item := range result.Items { - conn, err := services.UnmarshalSAMLConnector( - item.Value, services.WithExpires(item.Expires)) + conn, err := services.UnmarshalSAMLConnector(item.Value, services.WithExpires(item.Expires), services.WithRevision(item.Revision)) if err != nil { return nil, trace.Wrap(err) } @@ -1276,13 +1361,61 @@ func (s *IdentityService) UpsertGithubConnector(ctx context.Context, connector t ID: connector.GetResourceID(), Revision: rev, } - _, err = s.Put(ctx, item) + lease, err := s.Put(ctx, item) if err != nil { return trace.Wrap(err) } + connector.SetRevision(lease.Revision) return nil } +// UpdateGithubConnector updates an existing Github connector. +func (s *IdentityService) UpdateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + if err := connector.CheckAndSetDefaults(); err != nil { + return nil, trace.Wrap(err) + } + value, err := services.MarshalGithubConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + item := backend.Item{ + Key: backend.Key(webPrefix, connectorsPrefix, githubPrefix, connectorsPrefix, connector.GetName()), + Value: value, + Expires: connector.Expiry(), + ID: connector.GetResourceID(), + Revision: connector.GetRevision(), + } + lease, err := s.ConditionalUpdate(ctx, item) + if err != nil { + return nil, trace.Wrap(err) + } + connector.SetRevision(lease.Revision) + return connector, nil +} + +// CreateGithubConnector creates a new Github connector. +func (s *IdentityService) CreateGithubConnector(ctx context.Context, connector types.GithubConnector) (types.GithubConnector, error) { + if err := connector.CheckAndSetDefaults(); err != nil { + return nil, trace.Wrap(err) + } + value, err := services.MarshalGithubConnector(connector) + if err != nil { + return nil, trace.Wrap(err) + } + item := backend.Item{ + Key: backend.Key(webPrefix, connectorsPrefix, githubPrefix, connectorsPrefix, connector.GetName()), + Value: value, + Expires: connector.Expiry(), + ID: connector.GetResourceID(), + } + lease, err := s.Create(ctx, item) + if err != nil { + return nil, trace.Wrap(err) + } + connector.SetRevision(lease.Revision) + return connector, nil +} + // GetGithubConnectors returns all configured Github connectors func (s *IdentityService) GetGithubConnectors(ctx context.Context, withSecrets bool) ([]types.GithubConnector, error) { startKey := backend.ExactKey(webPrefix, connectorsPrefix, githubPrefix, connectorsPrefix) @@ -1292,7 +1425,7 @@ func (s *IdentityService) GetGithubConnectors(ctx context.Context, withSecrets b } connectors := make([]types.GithubConnector, len(result.Items)) for i, item := range result.Items { - connector, err := services.UnmarshalGithubConnector(item.Value) + connector, err := services.UnmarshalGithubConnector(item.Value, services.WithRevision(item.Revision)) if err != nil { return nil, trace.Wrap(err) } @@ -1316,7 +1449,7 @@ func (s *IdentityService) GetGithubConnector(ctx context.Context, name string, w } return nil, trace.Wrap(err) } - connector, err := services.UnmarshalGithubConnector(item.Value) + connector, err := services.UnmarshalGithubConnector(item.Value, services.WithRevision(item.Revision)) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/services/suite/suite.go b/lib/services/suite/suite.go index 380c0e36914..d224d6fe3f1 100644 --- a/lib/services/suite/suite.go +++ b/lib/services/suite/suite.go @@ -40,7 +40,9 @@ import ( "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/lib/auth/testauthority" + "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/fixtures" "github.com/gravitational/teleport/lib/services" @@ -746,6 +748,7 @@ func (s *ServicesTestSuite) SAMLCRUD(t *testing.T) { Namespace: apidefaults.Namespace, }, Spec: types.SAMLConnectorSpecV2{ + Display: "SAML", Issuer: "http://example.com", SSO: "https://example.com/saml/sso", AssertionConsumerService: "https://localhost/acs", @@ -767,21 +770,21 @@ func (s *ServicesTestSuite) SAMLCRUD(t *testing.T) { require.NoError(t, err) out, err := s.WebS.GetSAMLConnector(ctx, connector.GetName(), true) require.NoError(t, err) - require.Empty(t, cmp.Diff(out, connector)) + require.Empty(t, cmp.Diff(out, connector, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) connectors, err := s.WebS.GetSAMLConnectors(ctx, true) require.NoError(t, err) - require.Empty(t, cmp.Diff([]types.SAMLConnector{connector}, connectors)) + require.Empty(t, cmp.Diff([]types.SAMLConnector{connector}, connectors, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) out2, err := s.WebS.GetSAMLConnector(ctx, connector.GetName(), false) require.NoError(t, err) connectorNoSecrets := *connector connectorNoSecrets.Spec.SigningKeyPair.PrivateKey = "" - require.Empty(t, cmp.Diff(out2, &connectorNoSecrets)) + require.Empty(t, cmp.Diff(out2, &connectorNoSecrets, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) connectorsNoSecrets, err := s.WebS.GetSAMLConnectors(ctx, false) require.NoError(t, err) - require.Empty(t, cmp.Diff([]types.SAMLConnector{&connectorNoSecrets}, connectorsNoSecrets)) + require.Empty(t, cmp.Diff([]types.SAMLConnector{&connectorNoSecrets}, connectorsNoSecrets, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) err = s.WebS.DeleteSAMLConnector(ctx, connector.GetName()) require.NoError(t, err) @@ -791,6 +794,127 @@ func (s *ServicesTestSuite) SAMLCRUD(t *testing.T) { _, err = s.WebS.GetSAMLConnector(ctx, connector.GetName(), true) require.Equal(t, trace.IsNotFound(err), true, fmt.Sprintf("expected not found, got %T", err)) + + created, err := s.WebS.CreateSAMLConnector(ctx, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, created, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, created.GetRevision()) + + connector = utils.CloneProtoMsg(connector) + connector.SetRevision(uuid.NewString()) + connector.SetDisplay("not-saml") + + updated, err := s.WebS.UpdateSAMLConnector(ctx, connector) + require.ErrorIs(t, err, backend.ErrIncorrectRevision) + require.Nil(t, updated) + + connector.SetRevision(created.GetRevision()) + updated, err = s.WebS.UpdateSAMLConnector(ctx, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, updated, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, updated.GetRevision()) + require.NotEqual(t, created.GetRevision(), updated.GetRevision()) + require.NotEqual(t, created.GetDisplay(), updated.GetDisplay()) + + connector = utils.CloneProtoMsg(connector) + connector.SetRevision(uuid.NewString()) + connector.SetDisplay("llama") + + require.NoError(t, s.WebS.UpsertSAMLConnector(ctx, connector)) + upserted, err := s.WebS.GetSAMLConnector(ctx, connector.GetName(), true) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, upserted, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, upserted.GetRevision()) + require.NotEqual(t, updated.GetRevision(), upserted.GetRevision()) + require.NotEqual(t, updated.GetDisplay(), upserted.GetDisplay()) +} + +func (s *ServicesTestSuite) OIDCCRUD(t *testing.T) { + ctx := context.Background() + connector := &types.OIDCConnectorV3{ + Kind: types.KindOIDC, + Version: types.V2, + Metadata: types.Metadata{ + Name: "oidc1", + Namespace: apidefaults.Namespace, + }, + Spec: types.OIDCConnectorSpecV3{ + Display: "SAML", + IssuerURL: "http://example.com", + ClientID: "aaa", + ClientSecret: "bbb", + RedirectURLs: []string{"https://localhost:3080/v1/webapi/github/callback"}, + ClaimsToRoles: []types.ClaimMapping{ + { + Claim: "abc", + Value: "xyz", + Roles: []string{"admin"}, + }, + }, + }, + } + + err := s.WebS.UpsertOIDCConnector(ctx, connector) + require.NoError(t, err) + out, err := s.WebS.GetOIDCConnector(ctx, connector.GetName(), true) + require.NoError(t, err) + require.Empty(t, cmp.Diff(out, connector, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + + connectors, err := s.WebS.GetOIDCConnectors(ctx, true) + require.NoError(t, err) + require.Empty(t, cmp.Diff([]types.OIDCConnector{connector}, connectors, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + + out2, err := s.WebS.GetOIDCConnector(ctx, connector.GetName(), false) + require.NoError(t, err) + connectorNoSecrets := *connector + connectorNoSecrets.Spec.ClientSecret = "" + require.Empty(t, cmp.Diff(out2, &connectorNoSecrets, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + + connectorsNoSecrets, err := s.WebS.GetOIDCConnectors(ctx, false) + require.NoError(t, err) + require.Empty(t, cmp.Diff([]types.OIDCConnector{&connectorNoSecrets}, connectorsNoSecrets, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + + err = s.WebS.DeleteOIDCConnector(ctx, connector.GetName()) + require.NoError(t, err) + + err = s.WebS.DeleteOIDCConnector(ctx, connector.GetName()) + require.Equal(t, trace.IsNotFound(err), true, fmt.Sprintf("expected not found, got %T", err)) + + _, err = s.WebS.GetOIDCConnector(ctx, connector.GetName(), true) + require.Equal(t, trace.IsNotFound(err), true, fmt.Sprintf("expected not found, got %T", err)) + + created, err := s.WebS.CreateOIDCConnector(ctx, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, created, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, created.GetRevision()) + + connector = utils.CloneProtoMsg(connector) + connector.SetRevision(uuid.NewString()) + connector.SetDisplay("not-saml") + + updated, err := s.WebS.UpdateOIDCConnector(ctx, connector) + require.ErrorIs(t, err, backend.ErrIncorrectRevision) + require.Nil(t, updated) + + connector.SetRevision(created.GetRevision()) + updated, err = s.WebS.UpdateOIDCConnector(ctx, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, updated, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, updated.GetRevision()) + require.NotEqual(t, created.GetRevision(), updated.GetRevision()) + require.NotEqual(t, created.GetDisplay(), updated.GetDisplay()) + + connector = utils.CloneProtoMsg(connector) + connector.SetRevision(uuid.NewString()) + connector.SetDisplay("llama") + + require.NoError(t, s.WebS.UpsertOIDCConnector(ctx, connector)) + upserted, err := s.WebS.GetOIDCConnector(ctx, connector.GetName(), true) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, upserted, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, upserted.GetRevision()) + require.NotEqual(t, updated.GetRevision(), upserted.GetRevision()) + require.NotEqual(t, updated.GetDisplay(), upserted.GetDisplay()) } func (s *ServicesTestSuite) TunnelConnectionsCRUD(t *testing.T) { @@ -872,12 +996,11 @@ func (s *ServicesTestSuite) GithubConnectorCRUD(t *testing.T) { ClientSecret: "bbb", RedirectURL: "https://localhost:3080/v1/webapi/github/callback", Display: "GitHub", - TeamsToLogins: []types.TeamMapping{ + TeamsToRoles: []types.TeamRolesMapping{ { Organization: "gravitational", Team: "admins", - Logins: []string{"admin"}, - KubeGroups: []string{"system:masters"}, + Roles: []string{"admin"}, }, }, }, @@ -888,21 +1011,21 @@ func (s *ServicesTestSuite) GithubConnectorCRUD(t *testing.T) { require.NoError(t, err) out, err := s.WebS.GetGithubConnector(ctx, connector.GetName(), true) require.NoError(t, err) - require.Empty(t, cmp.Diff(out, connector)) + require.Empty(t, cmp.Diff(out, connector, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) connectors, err := s.WebS.GetGithubConnectors(ctx, true) require.NoError(t, err) - require.Empty(t, cmp.Diff([]types.GithubConnector{connector}, connectors)) + require.Empty(t, cmp.Diff([]types.GithubConnector{connector}, connectors, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) out2, err := s.WebS.GetGithubConnector(ctx, connector.GetName(), false) require.NoError(t, err) connectorNoSecrets := *connector connectorNoSecrets.Spec.ClientSecret = "" - require.Empty(t, cmp.Diff(out2, &connectorNoSecrets)) + require.Empty(t, cmp.Diff(out2, &connectorNoSecrets, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) connectorsNoSecrets, err := s.WebS.GetGithubConnectors(ctx, false) require.NoError(t, err) - require.Empty(t, cmp.Diff([]types.GithubConnector{&connectorNoSecrets}, connectorsNoSecrets)) + require.Empty(t, cmp.Diff([]types.GithubConnector{&connectorNoSecrets}, connectorsNoSecrets, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) err = s.WebS.DeleteGithubConnector(ctx, connector.GetName()) require.NoError(t, err) @@ -912,6 +1035,40 @@ func (s *ServicesTestSuite) GithubConnectorCRUD(t *testing.T) { _, err = s.WebS.GetGithubConnector(ctx, connector.GetName(), true) require.Equal(t, trace.IsNotFound(err), true, fmt.Sprintf("expected not found, got %T", err)) + + created, err := s.WebS.CreateGithubConnector(ctx, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, created, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, created.GetRevision()) + + connector = utils.CloneProtoMsg(connector) + connector.SetRevision(uuid.NewString()) + connector.SetDisplay("not-github") + + updated, err := s.WebS.UpdateGithubConnector(ctx, connector) + require.ErrorIs(t, err, backend.ErrIncorrectRevision) + require.Nil(t, updated) + + connector.SetRevision(created.GetRevision()) + updated, err = s.WebS.UpdateGithubConnector(ctx, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, updated, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, updated.GetRevision()) + require.NotEqual(t, created.GetRevision(), updated.GetRevision()) + require.NotEqual(t, created.GetDisplay(), updated.GetDisplay()) + + connector = utils.CloneProtoMsg(connector) + connector.SetRevision(uuid.NewString()) + connector.SetDisplay("llama") + + require.NoError(t, s.WebS.UpsertGithubConnector(ctx, connector)) + upserted, err := s.WebS.GetGithubConnector(ctx, connector.GetName(), true) + require.NoError(t, err) + require.Empty(t, cmp.Diff(connector, upserted, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"))) + require.NotEmpty(t, upserted.GetRevision()) + require.NotEqual(t, updated.GetRevision(), upserted.GetRevision()) + require.NotEqual(t, updated.GetDisplay(), upserted.GetDisplay()) + } func (s *ServicesTestSuite) RemoteClustersCRUD(t *testing.T) {