diff --git a/integrations/access/servicenow/app.go b/integrations/access/servicenow/app.go index 4ec2eadd56a85..6189e6a9b7ea0 100644 --- a/integrations/access/servicenow/app.go +++ b/integrations/access/servicenow/app.go @@ -18,7 +18,6 @@ package servicenow import ( "context" - "errors" "fmt" "net/url" "strings" @@ -29,6 +28,7 @@ import ( "golang.org/x/exp/slices" tp "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/accessrequest" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/integrations/access/common" @@ -54,12 +54,10 @@ const ( modifyPluginDataBackoffMax = time.Second ) -// errMissingAnnotation is used for cases where request annotations are not set -var errMissingAnnotation = errors.New("access request is missing annotations") - // App is a wrapper around the base app to allow for extra functionality. type App struct { *lib.Process + common.BaseApp PluginName string teleport teleport.Client @@ -137,6 +135,7 @@ func (a *App) run(ctx context.Context) error { func (a *App) init(ctx context.Context) error { ctx, cancel := context.WithTimeout(ctx, initTimeout) defer cancel() + log := logger.Get(ctx) var err error if a.teleport == nil { @@ -160,6 +159,13 @@ func (a *App) init(ctx context.Context) error { if err != nil { return trace.Wrap(err) } + + log.Debug("Starting API health check...") + if err = a.serviceNow.CheckHealth(ctx); err != nil { + return trace.Wrap(err, "API health check failed") + } + log.Debug("API health check finished ok") + return nil } @@ -227,27 +233,57 @@ func (a *App) onWatcherEvent(ctx context.Context, event types.Event) error { } func (a *App) onPendingRequest(ctx context.Context, req types.AccessRequest) error { - if len(req.GetSystemAnnotations()) == 0 { - logger.Get(ctx).Debug("Cannot proceed further. Request is missing any annotations") - return nil + reqID := req.GetName() + log := logger.Get(ctx).WithField("reqId", reqID) + + resourceNames, err := a.getResourceNames(ctx, req) + if err != nil { + return trace.Wrap(err) } - // First, try to create a notification incident. - isNew, notifyErr := a.tryNotifyService(ctx, req) + reqData := RequestData{ + User: req.GetUser(), + Roles: req.GetRoles(), + RequestReason: req.GetRequestReason(), + SystemAnnotations: req.GetSystemAnnotations(), + Resources: resourceNames, + } + // Create plugin data if it didn't exist before. + isNew, err := a.modifyPluginData(ctx, reqID, func(existing *PluginData) (PluginData, bool) { + if existing != nil { + return PluginData{}, false + } + return PluginData{RequestData: reqData}, true + }) + if err != nil { + return trace.Wrap(err, "updating plugin data") + } + + if isNew { + log.Infof("Creating servicenow incident") + if err = a.createIncident(ctx, reqID, reqData); err != nil { + // Even if we failed to create the incident we try to auto-approve + return trace.NewAggregate( + trace.WrapWithMessage(err, "creating ServiceNow incident"), + trace.Wrap(a.tryApproveRequest(ctx, req)), + ) + } + } + if reqReviews := req.GetReviews(); len(reqReviews) > 0 { + if err = a.postReviewNotes(ctx, reqID, reqReviews); err != nil { + return trace.NewAggregate( + trace.WrapWithMessage(err, "posting review notes"), + trace.Wrap(a.tryApproveRequest(ctx, req)), + ) + } + } // To minimize the count of auto-approval tries, let's only attempt it only when we have just created an incident. - // But if there's an error, we can't really know if the incident is new or not so lets just try. - if !isNew && notifyErr == nil { + if !isNew { return nil } - // Don't show the error if the annotation is just missing. - if errors.Is(notifyErr, errMissingAnnotation) { - notifyErr = nil - } - - // Then, try to approve the request if user is currently on-call. - approveErr := trace.Wrap(a.tryApproveRequest(ctx, req)) - return trace.NewAggregate(notifyErr, approveErr) + // Try to approve the request if user is currently on-call. + return trace.Wrap(a.tryApproveRequest(ctx, req)) } func (a *App) onResolvedRequest(ctx context.Context, req types.AccessRequest) error { @@ -278,14 +314,6 @@ func (a *App) onDeletedRequest(ctx context.Context, reqID string) error { return a.resolveIncident(ctx, reqID, Resolution{State: ResolutionStateResolved}) } -func (a *App) getNotifyServiceNames(req types.AccessRequest) ([]string, error) { - services, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationNotifyServicesLabel] - if !ok { - return nil, trace.NotFound("notify services not specified") - } - return services, nil -} - func (a *App) getOnCallServiceNames(req types.AccessRequest) ([]string, error) { services, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationSchedulesLabel] if !ok { @@ -294,54 +322,8 @@ func (a *App) getOnCallServiceNames(req types.AccessRequest) ([]string, error) { return services, nil } -func (a *App) tryNotifyService(ctx context.Context, req types.AccessRequest) (bool, error) { - log := logger.Get(ctx) - - serviceNames, err := a.getNotifyServiceNames(req) - if err != nil { - log.Debugf("Skipping the notification: %s", err) - return false, trace.Wrap(errMissingAnnotation) - } - - reqID := req.GetName() - reqData := RequestData{ - User: req.GetUser(), - Roles: req.GetRoles(), - Created: req.GetCreationTime(), - RequestReason: req.GetRequestReason(), - } - - // Create plugin data if it didn't exist before. - isNew, err := a.modifyPluginData(ctx, reqID, func(existing *PluginData) (PluginData, bool) { - if existing != nil { - return PluginData{}, false - } - return PluginData{RequestData: reqData}, true - }) - if err != nil { - return isNew, trace.Wrap(err, "updating plugin data") - } - - if isNew { - for _, serviceName := range serviceNames { - incidentCtx, _ := logger.WithField(ctx, "servicenow_service_name", serviceName) - - if err = a.createIncident(incidentCtx, serviceName, reqID, reqData); err != nil { - return isNew, trace.Wrap(err, "creating ServiceNow incident") - } - } - - if reqReviews := req.GetReviews(); len(reqReviews) > 0 { - if err = a.postReviewNotes(ctx, reqID, reqReviews); err != nil { - return isNew, trace.Wrap(err) - } - } - } - return isNew, nil -} - // createIncident posts an incident with request information. -func (a *App) createIncident(ctx context.Context, serviceID, reqID string, reqData RequestData) error { +func (a *App) createIncident(ctx context.Context, reqID string, reqData RequestData) error { data, err := a.serviceNow.CreateIncident(ctx, reqID, reqData) if err != nil { return trace.Wrap(err) @@ -431,7 +413,7 @@ func (a *App) tryApproveRequest(ctx context.Context, req types.AccessRequest) er if _, err := a.teleport.SubmitAccessReview(ctx, types.AccessReviewSubmission{ RequestID: req.GetName(), Review: types.AccessReview{ - Author: tp.SystemAccessApproverUserName, + Author: a.conf.TeleportUser, ProposedState: types.RequestState_APPROVED, Reason: fmt.Sprintf("Access requested by user %s who is on call on service(s) %s", tp.SystemAccessApproverUserName, @@ -574,3 +556,25 @@ func (a *App) updatePluginData(ctx context.Context, reqID string, data PluginDat Expect: EncodePluginData(expectData), }) } + +// getResourceNames returns the names of the requested resources. +func (a *App) getResourceNames(ctx context.Context, req types.AccessRequest) ([]string, error) { + resourceNames := make([]string, 0, len(req.GetRequestedResourceIDs())) + resourcesByCluster := accessrequest.GetResourceIDsByCluster(req) + + for cluster, resources := range resourcesByCluster { + resourceDetails, err := accessrequest.GetResourceDetails(ctx, cluster, a.teleport, resources) + if err != nil { + return nil, trace.Wrap(err) + } + + for _, resource := range resources { + resourceName := types.ResourceIDToString(resource) + if details, ok := resourceDetails[resourceName]; ok && details.FriendlyName != "" { + resourceName = fmt.Sprintf("%s/%s", resource.Kind, details.FriendlyName) + } + resourceNames = append(resourceNames, resourceName) + } + } + return resourceNames, nil +} diff --git a/integrations/access/servicenow/bot.go b/integrations/access/servicenow/bot.go index 8e0a163fc587e..96c48f6b0f084 100644 --- a/integrations/access/servicenow/bot.go +++ b/integrations/access/servicenow/bot.go @@ -57,7 +57,7 @@ func (b Bot) SendReviewReminders(ctx context.Context, recipients []common.Recipi } // BroadcastAccessRequestMessage creates a ServiceNow incident. -func (b *Bot) BroadcastAccessRequestMessage(ctx context.Context, recipients []common.Recipient, reqID string, reqData pd.AccessRequestData) (data accessrequest.SentMessages, err error) { +func (b *Bot) BroadcastAccessRequestMessage(ctx context.Context, _ []common.Recipient, reqID string, reqData pd.AccessRequestData) (data accessrequest.SentMessages, err error) { serviceNowReqData := RequestData{ User: reqData.User, Roles: reqData.Roles, diff --git a/integrations/access/servicenow/client.go b/integrations/access/servicenow/client.go index 43522efe2f73c..7fcf177142d7c 100644 --- a/integrations/access/servicenow/client.go +++ b/integrations/access/servicenow/client.go @@ -224,15 +224,15 @@ func (snc *Client) GetOnCall(ctx context.Context, rotaID string) ([]string, erro if len(result.Result) == 0 { return nil, trace.NotFound("no user found for given rota: %q", rotaID) } - var emails []string + var userNames []string for _, result := range result.Result { - email, err := snc.GetUserEmail(ctx, result.UserID) + userName, err := snc.GetUserName(ctx, result.UserID) if err != nil { return nil, trace.Wrap(err) } - emails = append(emails, email) + userNames = append(userNames, userName) } - return emails, nil + return userNames, nil } // CheckHealth pings servicenow to check if it is reachable. @@ -270,13 +270,13 @@ func (snc *Client) CheckHealth(ctx context.Context) error { return nil } -// GetUserEmail returns the email address for the given user ID -func (snc *Client) GetUserEmail(ctx context.Context, userID string) (string, error) { +// GetUserName returns the name for the given user ID +func (snc *Client) GetUserName(ctx context.Context, userID string) (string, error) { var result userResult resp, err := snc.client.NewRequest(). SetContext(ctx). SetQueryParams(map[string]string{ - "sysparm_fields": "email", + "sysparm_fields": "user_name", }). SetPathParams(map[string]string{"user_id": userID}). SetResult(&result). @@ -288,13 +288,10 @@ func (snc *Client) GetUserEmail(ctx context.Context, userID string) (string, err if resp.IsError() { return "", errWrapper(resp.StatusCode(), string(resp.Body())) } - if len(result.Result) == 0 { - return "", trace.NotFound("no user found for given id") - } - if len(result.Result) != 1 { - return "", trace.NotFound("more than one user returned for given id") + if result.Result.UserName == "" { + return "", trace.NotFound("no username found for given id: %v", userID) } - return result.Result[0].Email, nil + return result.Result.UserName, nil } var ( diff --git a/integrations/access/servicenow/config.go b/integrations/access/servicenow/config.go index fe38626d9c93a..40dd88f5a31dd 100644 --- a/integrations/access/servicenow/config.go +++ b/integrations/access/servicenow/config.go @@ -23,6 +23,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/integrations/access/common" + "github.com/gravitational/teleport/integrations/access/common/teleport" "github.com/gravitational/teleport/integrations/lib" ) @@ -31,6 +32,15 @@ type Config struct { common.BaseConfig ClientConfig ServiceNow common.GenericAPIConfig + + // Teleport is a handle to the client to use when communicating with + // the Teleport auth server. The ServiceNow app will create a gRPC-based + // client on startup if this is not set. + Client teleport.Client + + // TeleportUser is the name of the Teleport user that will act + // as the access request approver + TeleportUser string } // CheckAndSetDefaults checks the config struct for any logical errors, and sets default values diff --git a/integrations/access/servicenow/fake_servicenow_test.go b/integrations/access/servicenow/fake_servicenow_test.go index aceb5f76f11bd..26eb25b4331c5 100644 --- a/integrations/access/servicenow/fake_servicenow_test.go +++ b/integrations/access/servicenow/fake_servicenow_test.go @@ -67,7 +67,7 @@ type FakeIncident struct { Incident } -func NewFakeServiceNow(concurrency int) *FakeServiceNow { +func NewFakeServiceNow(concurrency int, onCallUser string) *FakeServiceNow { router := httprouter.New() serviceNow := &FakeServiceNow{ @@ -76,7 +76,7 @@ func NewFakeServiceNow(concurrency int) *FakeServiceNow { newIncidentNotes: make(chan string, concurrency*3), // for any incident there could be 1-3 notes srv: httptest.NewServer(router), } - + router.GET("/api/now/table/incident", func(rw http.ResponseWriter, r *http.Request, ps httprouter.Params) {}) router.POST("/api/now/v1/table/incident", func(rw http.ResponseWriter, r *http.Request, ps httprouter.Params) { rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusCreated) @@ -136,6 +136,27 @@ func NewFakeServiceNow(concurrency int) *FakeServiceNow { serviceNow.StoreIncident(incident) serviceNow.incidentUpdates <- incident }) + router.GET("/api/now/on_call_rota/whoisoncall", func(rw http.ResponseWriter, r *http.Request, ps httprouter.Params) { + rw.Header().Add("Content-Type", "application/json") + + err := json.NewEncoder(rw).Encode(onCallResult{Result: []struct { + UserID string `json:"userId"` + }{ + { + UserID: "someUserID", + }, + }}) + panicIf(err) + }) + router.GET("/api/now/table/sys_user/:UserID", func(rw http.ResponseWriter, r *http.Request, ps httprouter.Params) { + rw.Header().Add("Content-Type", "application/json") + err := json.NewEncoder(rw).Encode(userResult{Result: struct { + UserName string `json:"user_name"` + }{ + UserName: onCallUser, + }}) + panicIf(err) + }) return serviceNow } diff --git a/integrations/access/servicenow/servicenow_test.go b/integrations/access/servicenow/servicenow_test.go index 988bb86f19479..9263bed9d7e1b 100644 --- a/integrations/access/servicenow/servicenow_test.go +++ b/integrations/access/servicenow/servicenow_test.go @@ -36,11 +36,11 @@ import ( ) const ( - NotifyServiceName = "Teleport Notifications" - NotifyServiceAnnotation = types.TeleportNamespace + types.ReqAnnotationNotifyServicesLabel - ResponderName1 = "ResponderID1" - ResponderName2 = "RespondeID2" - ResponderName3 = "RespondeID3" + ScheduleAnnotation = types.TeleportNamespace + types.ReqAnnotationSchedulesLabel + Schedule = "someRotaID" + ResponderName1 = "ResponderID1" + ResponderName2 = "RespondeID2" + ResponderName3 = "RespondeID3" ) type ServiceNowSuite struct { @@ -48,22 +48,22 @@ type ServiceNowSuite struct { appConfig Config currentRequestor string userNames struct { - ruler string - reviewer1 string - reviewer2 string - requestor string - approver string - racer1 string - racer2 string - plugin string + ruler string + reviewer1 string + reviewer2 string + requestor string + requestorWithSchedules string + approver string + racer1 string + racer2 string + plugin string } raceNumber int fakeServiceNow *FakeServiceNow - snNotifyResponder string - snResponder1 string - snResponder2 string - snResponder3 string + snResponder1 string + snResponder2 string + snResponder3 string clients map[string]*integration.Client teleportFeatures *proto.Features @@ -117,9 +117,6 @@ func (s *ServiceNowSuite) SetupSuite() { conditions := types.RoleConditions{ Request: &types.AccessRequestConditions{ Roles: []string{"editor"}, - Annotations: wrappers.Traits{ - NotifyServiceAnnotation: []string{NotifyServiceName}, - }, }, } if teleportFeatures.AdvancedAccessWorkflows { @@ -133,6 +130,23 @@ func (s *ServiceNowSuite) SetupSuite() { require.NoError(t, err) s.userNames.requestor = user.GetName() + // Set up user who can request the access to role "editor" but with schedule annotation. + conditionsWithSchedule := types.RoleConditions{ + Request: &types.AccessRequestConditions{ + Roles: []string{"editor"}, + Annotations: wrappers.Traits{ + ScheduleAnnotation: []string{Schedule}, + }, + }, + } + // This is the role for testing notification incident creation With schedule. + roleWithSchedule, err := bootstrap.AddRole("fooWithSchedule", types.RoleSpecV6{Allow: conditionsWithSchedule}) + require.NoError(t, err) + + userWithSchedule, err := bootstrap.AddUserWithRoles(me.Username+"-schedule@example.com", roleWithSchedule.GetName()) + require.NoError(t, err) + s.userNames.requestorWithSchedules = userWithSchedule.GetName() + if teleportFeatures.AdvancedAccessWorkflows { // Set up TWO users who can review access requests to role "editor". @@ -152,13 +166,13 @@ func (s *ServiceNowSuite) SetupSuite() { s.userNames.reviewer2 = user.GetName() // This is the role that needs exactly one approval review for an access request to be approved. - // It's handy to test auto-approval scenarios so we also put "servicenow_services" annotation. + // It's handy to test auto-approval scenarios so we also set the schedule annotation. role, err = bootstrap.AddRole("bar", types.RoleSpecV6{ Allow: types.RoleConditions{ Request: &types.AccessRequestConditions{ Roles: []string{"editor"}, Annotations: wrappers.Traits{ - NotifyServiceAnnotation: []string{ResponderName1, ResponderName2}, + ScheduleAnnotation: []string{Schedule}, }, }, }, @@ -169,16 +183,11 @@ func (s *ServiceNowSuite) SetupSuite() { require.NoError(t, err) s.userNames.approver = user.GetName() - // This is the role with a maximum possible setup: both "servicenow_notify_service" and - // "servicenow_services" annotations and threshold. role, err = bootstrap.AddRole("foo-bar", types.RoleSpecV6{ Allow: types.RoleConditions{ Request: &types.AccessRequestConditions{ - Roles: []string{"editor"}, - Annotations: wrappers.Traits{ - NotifyServiceAnnotation: []string{NotifyServiceName}, - }, - Thresholds: []types.AccessReviewThreshold{types.AccessReviewThreshold{Approve: 2, Deny: 2}}, + Roles: []string{"editor"}, + Thresholds: []types.AccessReviewThreshold{{Approve: 2, Deny: 2}}, }, }, }) @@ -224,6 +233,10 @@ func (s *ServiceNowSuite) SetupSuite() { s.clients[s.userNames.requestor] = client if teleportFeatures.AdvancedAccessWorkflows { + client, err = teleport.NewClient(ctx, auth, s.userNames.requestorWithSchedules) + require.NoError(t, err) + s.clients[s.userNames.requestorWithSchedules] = client + client, err = teleport.NewClient(ctx, auth, s.userNames.approver) require.NoError(t, err) s.clients[s.userNames.approver] = client @@ -259,11 +272,10 @@ func (s *ServiceNowSuite) SetupTest() { err := logger.Setup(logger.Config{Severity: "debug"}) require.NoError(t, err) - fakeServiceNow := NewFakeServiceNow(s.raceNumber) + fakeServiceNow := NewFakeServiceNow(s.raceNumber, s.userNames.requestorWithSchedules) t.Cleanup(fakeServiceNow.Close) s.fakeServiceNow = fakeServiceNow - s.snNotifyResponder = s.fakeServiceNow.StoreResponder(s.Context(), NotifyServiceName) s.snResponder1 = s.fakeServiceNow.StoreResponder(s.Context(), ResponderName1) s.snResponder2 = s.fakeServiceNow.StoreResponder(s.Context(), ResponderName2) s.snResponder3 = s.fakeServiceNow.StoreResponder(s.Context(), ResponderName3) @@ -309,9 +321,6 @@ func (s *ServiceNowSuite) newAccessRequest() types.AccessRequest { t.Helper() req, err := types.NewAccessRequest(uuid.New().String(), s.currentRequestor, "editor") - req.SetSystemAnnotations(map[string][]string{ - NotifyServiceAnnotation: {NotifyServiceName}, - }) require.NoError(s.T(), err) return req } @@ -545,3 +554,23 @@ func (s *ServiceNowSuite) TestDenialByReview() { assert.Contains(t, incident.CloseNotes, "Reason: finally not okay") assert.Equal(t, "resolved", incident.CloseCode) } + +func (s *ServiceNowSuite) TestAutoApproval() { + t := s.T() + + if !s.teleportFeatures.AdvancedAccessWorkflows { + t.Skip("Doesn't work in OSS version") + } + + s.startApp() + + s.currentRequestor = s.userNames.requestorWithSchedules + _ = s.createAccessRequest() + + _, err := s.fakeServiceNow.CheckNewIncident(s.Context()) + require.NoError(t, err, "no new incidents stored") + + incident, err := s.fakeServiceNow.CheckIncidentUpdate(s.Context()) + require.NoError(t, err) + assert.Contains(t, incident.WorkNotes, "Resolution: APPROVED") +} diff --git a/integrations/access/servicenow/types.go b/integrations/access/servicenow/types.go index af40bdcac25e7..7c1b61ff3fe50 100644 --- a/integrations/access/servicenow/types.go +++ b/integrations/access/servicenow/types.go @@ -18,8 +18,6 @@ package servicenow import ( "time" - - "github.com/gravitational/teleport/api/types" ) // PluginData is a data associated with access request that we store in Teleport using UpdatePluginData API. @@ -89,7 +87,7 @@ type RequestData struct { // Resolution is the final resolution of the access request. Resolution Resolution // SystemAnnotations contains key value annotations for the request. - SystemAnnotations types.Labels + SystemAnnotations map[string][]string // Resources are the resources being requested. Resources []string // SuggestedReviewers are the suggested reviewers for this access request. @@ -104,9 +102,10 @@ type onCallResult struct { } type userResult struct { - Result []struct { - // Email is the email address in servicenow of the requested user. - Email string `json:"email"` + Result struct { + // UserName is the username in servicenow of the requested user. + // username chosen over email as identifier as it is guaranteed to be set. + UserName string `json:"user_name"` } `json:"result"` }