[v14] Add auto approval flow for servicenow plugin

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
+}

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,

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 (

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

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
 }