Merge 22f640c into 53b6f4b

CHANGELOG.md

@@ -6,6 +6,16 @@ We will list in this document any breaking changes between versions, that requir
 
 ## Breaking changes
 
+### v1.4.0
+##### API routes
+- `GET /resolution` API route has been __deleted__. We decided that resolutions should always be accessed through tasks
+
+##### Configuration
+- `resolver_usernames` configuration field __deleted__. Global resolver is not a feature we want to support, and its usage was misguided.
+
+##### SQL
+- `001_resolver_watcher_usernames_indexes.sql` migration file should be applied while upgrading. This migration is non-breaking but will boost performance on big instances for tasks listing.
+
 ### v1.3.0 (2020-03-17)
 ##### HTTP plugin
 - `timeout_seconds` configuration field __deleted__. It has been replaced by the field `timeout`, using the Golang time.Duration format (5s, 1m, ...), for consistency with other plugins and to express timeout durations inferior than 1 second. #86

README.md

@@ -159,8 +159,8 @@ The user that creates a task is called `requester`, and the user that executes i
 
 A user can be allowed to resolve a task in three ways:
 - the user is included in the global configuration's list of `admin_usernames`
-- the user is included in the global configuration's list of `resolver_usernames`
 - the user is included in the task's template list of `allowed_resolver_usernames`
+- the user is included in the task `resolver_usernames` list
 
 ### Value Templating
 

api/handler/batch.go

@@ -4,17 +4,18 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/juju/errors"
 	"github.com/loopfz/gadgeto/zesty"
+
 	"github.com/ovh/utask"
 	"github.com/ovh/utask/models/task"
 	"github.com/ovh/utask/models/tasktemplate"
+	"github.com/ovh/utask/pkg/taskutils"
 )
 
 type createBatchIn struct {
 	TemplateName     string                   `json:"template_name" binding:"required"`
 	CommonInput      map[string]interface{}   `json:"common_input"`
 	Inputs           []map[string]interface{} `json:"inputs" binding:"required"`
 	Comment          string                   `json:"comment"`
-	WatcherTokens    []string                 `json:"watcher_tokens"`
 	WatcherUsernames []string                 `json:"watcher_usernames"`
 }
 
@@ -49,7 +50,7 @@ func CreateBatch(c *gin.Context, in *createBatchIn) (*task.Batch, error) {
 			return nil, err
 		}
 
-		_, err = transactionTaskCreate(c, dbp, tt, in.WatcherUsernames, input, b, in.Comment, nil)
+		_, err = taskutils.CreateTask(c, dbp, tt, in.WatcherUsernames, []string{}, input, b, in.Comment, nil)
 		if err != nil {
 			dbp.Rollback()
 			return nil, err

api/handler/comment.go

@@ -4,9 +4,10 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/juju/errors"
 	"github.com/loopfz/gadgeto/zesty"
+
 	"github.com/ovh/utask"
+	"github.com/ovh/utask/models/resolution"
 	"github.com/ovh/utask/models/task"
-
 	"github.com/ovh/utask/models/tasktemplate"
 	"github.com/ovh/utask/pkg/auth"
 )
@@ -33,11 +34,19 @@ func CreateComment(c *gin.Context, in *createCommentIn) (*task.Comment, error) {
 		return nil, err
 	}
 
-	requester := isRequester(t, c) == nil
-	watcher := isWatcher(t, c) == nil
-	resolver := auth.IsAllowedResolver(c, tt, t.ResolverUsernames) == nil
+	var res *resolution.Resolution
+	if t.Resolution != nil {
+		res, err = resolution.LoadFromPublicID(dbp, *t.Resolution)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	requester := auth.IsRequester(c, t) == nil
+	watcher := auth.IsWatcher(c, t) == nil
+	resolutionManager := auth.IsResolutionManager(c, tt, t, res) == nil
 
-	if !requester && !watcher && !resolver {
+	if !requester && !watcher && !resolutionManager {
 		return nil, errors.Forbiddenf("Can't create comment")
 	}
 
@@ -68,6 +77,32 @@ func GetComment(c *gin.Context, in *getCommentIn) (*task.Comment, error) {
 		return nil, err
 	}
 
+	t, err := task.LoadFromPublicID(dbp, in.TaskID)
+	if err != nil {
+		return nil, err
+	}
+
+	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
+	if err != nil {
+		return nil, err
+	}
+
+	var res *resolution.Resolution
+	if t.Resolution != nil {
+		res, err = resolution.LoadFromPublicID(dbp, *t.Resolution)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	requester := auth.IsRequester(c, t) == nil
+	watcher := auth.IsWatcher(c, t) == nil
+	resolutionManager := auth.IsResolutionManager(c, tt, t, res) == nil
+
+	if !requester && !watcher && !resolutionManager {
+		return nil, errors.Forbiddenf("Can't get comment")
+	}
+
 	return comment, nil
 }
 
@@ -87,6 +122,27 @@ func ListComments(c *gin.Context, in *listCommentsIn) ([]*task.Comment, error) {
 		return nil, err
 	}
 
+	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
+	if err != nil {
+		return nil, err
+	}
+
+	var res *resolution.Resolution
+	if t.Resolution != nil {
+		res, err = resolution.LoadFromPublicID(dbp, *t.Resolution)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	requester := auth.IsRequester(c, t) == nil
+	watcher := auth.IsWatcher(c, t) == nil
+	resolutionManager := auth.IsResolutionManager(c, tt, t, res) == nil
+
+	if !requester && !watcher && !resolutionManager {
+		return nil, errors.Forbiddenf("Can't list comment")
+	}
+
 	comments, err := task.LoadCommentsFromTaskID(dbp, t.ID)
 	if err != nil {
 		return nil, err
@@ -120,7 +176,7 @@ func UpdateComment(c *gin.Context, in *updateCommentIn) (*task.Comment, error) {
 
 	reqUsername := auth.GetIdentity(c)
 
-	if reqUsername != comment.Username {
+	if auth.IsAdmin(c) != nil && reqUsername != comment.Username {
 		return nil, errors.Forbiddenf("Can't update comment")
 	}
 
@@ -160,7 +216,7 @@ func DeleteComment(c *gin.Context, in *deleteCommentIn) error {
 
 	reqUsername := auth.GetIdentity(c)
 
-	if reqUsername != comment.Username {
+	if auth.IsAdmin(c) != nil && reqUsername != comment.Username {
 		return errors.Forbiddenf("Can't update comment")
 	}
 

api/handler/resolution.go

@@ -48,13 +48,23 @@ func CreateResolution(c *gin.Context, in *createResolutionIn) (*resolution.Resol
 		return nil, err
 	}
 
-	if err := auth.IsAllowedResolver(c, tt, t.ResolverUsernames); err != nil {
+	if err := auth.IsResolutionManager(c, tt, t, nil); err != nil {
 		dbp.Rollback()
-		return nil, err
+		return nil, errors.Forbiddenf("You are not allowed to resolve this task")
 	}
 
 	resUser := auth.GetIdentity(c)
 
+	// adding current resolver to task.resolver_usernames, to be able to list resolved tasks
+	// as 'resolvable', if current resolver used admins privileges.
+	if auth.IsAdmin(c) == nil {
+		t.ResolverUsernames = append(t.ResolverUsernames, resUser)
+		if err := t.Update(dbp, false, false); err != nil {
+			dbp.Rollback()
+			return nil, err
+		}
+	}
+
 	r, err := resolution.Create(dbp, t, in.ResolverInputs, resUser, false, nil) // TODO accept delay in handler
 	if err != nil {
 		dbp.Rollback()
@@ -169,7 +179,7 @@ func UpdateResolution(c *gin.Context, in *updateResolutionIn) error {
 		return err
 	}
 
-	r, err := resolution.LoadFromPublicID(dbp, in.PublicID)
+	r, err := resolution.LoadLockedNoWaitFromPublicID(dbp, in.PublicID)
 	if err != nil {
 		dbp.Rollback()
 		return err
@@ -220,17 +230,28 @@ func RunResolution(c *gin.Context, in *runResolutionIn) error {
 		return err
 	}
 
+	// not using a LoadLocked here, because GetEngine().Resolve will lock row
 	r, err := resolution.LoadFromPublicID(dbp, in.PublicID)
 	if err != nil {
 		return err
 	}
 
-	logrus.WithFields(logrus.Fields{"resolution_id": r.PublicID}).Debugf("Handler RunResolution: manual resolve %s", r.PublicID)
+	t, err := task.LoadFromID(dbp, r.TaskID)
+	if err != nil {
+		return err
+	}
 
-	if err := auth.IsResolver(c, r); err != nil {
+	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
+	if err != nil {
 		return err
 	}
 
+	if err := auth.IsResolutionManager(c, tt, t, r); err != nil {
+		return errors.Forbiddenf("You are not allowed to resolve this task")
+	}
+
+	logrus.WithFields(logrus.Fields{"resolution_id": r.PublicID}).Debugf("Handler RunResolution: manual resolve %s", r.PublicID)
+
 	return engine.GetEngine().Resolve(in.PublicID)
 }
 
@@ -246,29 +267,38 @@ func ExtendResolution(c *gin.Context, in *extendResolutionIn) error {
 		return err
 	}
 
-	r, err := resolution.LoadFromPublicID(dbp, in.PublicID)
-	if err != nil {
+	if err := dbp.Tx(); err != nil {
 		return err
 	}
 
-	if err := auth.IsResolver(c, r); err != nil {
+	r, err := resolution.LoadLockedNoWaitFromPublicID(dbp, in.PublicID)
+	if err != nil {
+		dbp.Rollback()
 		return err
 	}
 
-	if r.State != resolution.StateBlockedMaxRetries {
-		return errors.NotValidf("Cannot extend a resolution which is not in state '%s'", resolution.StateBlockedMaxRetries)
-	}
-
 	t, err := task.LoadFromID(dbp, r.TaskID)
 	if err != nil {
+		dbp.Rollback()
 		return err
 	}
 
 	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
 	if err != nil {
+		dbp.Rollback()
 		return err
 	}
 
+	if err := auth.IsResolutionManager(c, tt, t, r); err != nil {
+		dbp.Rollback()
+		return err
+	}
+
+	if r.State != resolution.StateBlockedMaxRetries {
+		dbp.Rollback()
+		return errors.NotValidf("Cannot extend a resolution which is not in state '%s'", resolution.StateBlockedMaxRetries)
+	}
+
 	if tt.RetryMax != nil {
 		r.ExtendRunMax(*tt.RetryMax)
 	} else {
@@ -278,7 +308,17 @@ func ExtendResolution(c *gin.Context, in *extendResolutionIn) error {
 	r.SetState(resolution.StateError)
 	r.SetNextRetry(time.Now())
 
-	return r.Update(dbp)
+	if err := r.Update(dbp); err != nil {
+		dbp.Rollback()
+		return err
+	}
+
+	if err := dbp.Commit(); err != nil {
+		dbp.Rollback()
+		return err
+	}
+
+	return nil
 }
 
 type cancelResolutionIn struct {
@@ -297,13 +337,25 @@ func CancelResolution(c *gin.Context, in *cancelResolutionIn) error {
 		return err
 	}
 
-	r, err := resolution.LoadLockedFromPublicID(dbp, in.PublicID)
+	r, err := resolution.LoadLockedNoWaitFromPublicID(dbp, in.PublicID)
 	if err != nil {
 		dbp.Rollback()
 		return err
 	}
 
-	if err := auth.IsResolver(c, r); err != nil {
+	t, err := task.LoadFromPublicID(dbp, r.TaskPublicID)
+	if err != nil {
+		dbp.Rollback()
+		return err
+	}
+
+	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
+	if err != nil {
+		dbp.Rollback()
+		return err
+	}
+
+	if err := auth.IsResolutionManager(c, tt, t, r); err != nil {
 		dbp.Rollback()
 		return err
 	}
@@ -321,12 +373,6 @@ func CancelResolution(c *gin.Context, in *cancelResolutionIn) error {
 		return err
 	}
 
-	t, err := task.LoadLockedFromPublicID(dbp, r.TaskPublicID)
-	if err != nil {
-		dbp.Rollback()
-		return err
-	}
-
 	t.SetState(task.StateCancelled)
 
 	if err := t.Update(dbp, true, true); err != nil {
@@ -361,19 +407,32 @@ func PauseResolution(c *gin.Context, in *pauseResolutionIn) error {
 		return err
 	}
 
-	r, err := resolution.LoadLockedFromPublicID(dbp, in.PublicID)
+	r, err := resolution.LoadLockedNoWaitFromPublicID(dbp, in.PublicID)
 	if err != nil {
 		dbp.Rollback()
 		return err
 	}
 
-	if err := auth.IsResolver(c, r); err != nil {
+	t, err := task.LoadFromID(dbp, r.TaskID)
+	if err != nil {
 		dbp.Rollback()
 		return err
 	}
 
+	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
+	if err != nil {
+		dbp.Rollback()
+		return err
+	}
+
+	if err := auth.IsResolutionManager(c, tt, t, r); err != nil {
+		dbp.Rollback()
+		return errors.Forbiddenf("You are not allowed to resolve this task")
+	}
+
 	if in.Force {
 		if err := auth.IsAdmin(c); err != nil {
+			dbp.Rollback()
 			return err
 		}
 	} else {