Enable the delete workitem endpoint

controller/workitem.go

@@ -71,10 +71,8 @@ func NewNotifyingWorkitemController(service *goa.Service, db application.DB, not
 		config:       config}
 }
 
-// authorizeWorkitemTypeEditor returns true if the modifier is allowed to change
-// workitem type else it returns false.
-// Only space owner and workitem creator are allowed to change workitem type
-func (c *WorkitemController) authorizeWorkitemTypeEditor(ctx context.Context, spaceID uuid.UUID, creatorID string, editorID string) (bool, error) {
+// isWorkitemCreatorOrSpaceOwner returns true if the modifier is space owner or workitem creator
+func (c *WorkitemController) isWorkitemCreatorOrSpaceOwner(ctx context.Context, spaceID uuid.UUID, creatorID string, editorID string) (bool, error) {
 	// check if workitem editor is same as workitem creator
 	if editorID == creatorID {
 		return true, nil
@@ -87,7 +85,7 @@ func (c *WorkitemController) authorizeWorkitemTypeEditor(ctx context.Context, sp
 	if space != nil && editorID == space.OwnerID.String() {
 		return true, nil
 	}
-	return false, errors.NewUnauthorizedError("user is not allowed to change workitem type")
+	return false, errors.NewForbiddenError("user is not a workitem creator or space owner")
 }
 
 // Returns true if the user is the work item creator or space collaborator
@@ -134,7 +132,7 @@ func (c *WorkitemController) Update(ctx *app.UpdateWorkitemContext) error {
 	if ctx.Payload.Data.Relationships != nil && ctx.Payload.Data.Relationships.BaseType != nil &&
 		ctx.Payload.Data.Relationships.BaseType.Data != nil && ctx.Payload.Data.Relationships.BaseType.Data.ID != wi.Type {
 
-		authorized, err := c.authorizeWorkitemTypeEditor(ctx, wi.SpaceID, creator.(string), currentUserIdentityID.String())
+		authorized, err := c.isWorkitemCreatorOrSpaceOwner(ctx, wi.SpaceID, creator.(string), currentUserIdentityID.String())
 		if err != nil {
 			return jsonapi.JSONErrorResponse(ctx, err)
 		}
@@ -236,14 +234,11 @@ func (c *WorkitemController) Show(ctx *app.ShowWorkitemContext) error {
 
 // Delete does DELETE workitem
 func (c *WorkitemController) Delete(ctx *app.DeleteWorkitemContext) error {
-	// Temporarly disabled, See https://github.com/fabric8-services/fabric8-wit/issues/1036
-	if true {
-		return ctx.MethodNotAllowed()
-	}
 	currentUserIdentityID, err := login.ContextIdentity(ctx)
 	if err != nil {
 		return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError(err.Error()))
 	}
+
 	var wi *workitem.WorkItem
 	err = application.Transactional(c.db, func(appl application.Application) error {
 		wi, err = appl.WorkItems().LoadByID(ctx, ctx.WiID)
@@ -255,13 +250,25 @@ func (c *WorkitemController) Delete(ctx *app.DeleteWorkitemContext) error {
 	if err != nil {
 		return jsonapi.JSONErrorResponse(ctx, err)
 	}
-	authorized, err := authz.Authorize(ctx, wi.SpaceID.String())
+
+	// This checks if user is space admin or workitem creator or not
+	creator := wi.Fields[workitem.SystemCreator]
+	if creator == nil {
+		return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, errs.New("work item doesn't have creator")))
+	}
+	authorized, err := c.isWorkitemCreatorOrSpaceOwner(ctx, wi.SpaceID, creator.(string), currentUserIdentityID.String())
 	if err != nil {
-		return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError(err.Error()))
+		forbidden, err := errors.IsForbiddenError(err)
+		if forbidden {
+			return jsonapi.JSONErrorResponse(ctx, errors.NewForbiddenError("user is not authorized to delete the workitem"))
+
+		}
+		return err
 	}
 	if !authorized {
-		return jsonapi.JSONErrorResponse(ctx, errors.NewForbiddenError("user is not authorized to access the space"))
+		return jsonapi.JSONErrorResponse(ctx, errors.NewForbiddenError("user is not authorized to delete the workitem"))
 	}
+
 	err = application.Transactional(c.db, func(appl application.Application) error {
 		if err := appl.WorkItems().Delete(ctx, ctx.WiID, *currentUserIdentityID); err != nil {
 			return errs.Wrapf(err, "error deleting work item %s", ctx.WiID)

controller/workitem_blackbox_test.go

@@ -2304,17 +2304,6 @@ func (s *WorkItem2Suite) TestWI2FailShowMissing() {
 	test.ShowWorkitemNotFound(s.T(), s.svc.Context, s.svc, s.workitemCtrl, uuid.NewV4(), nil, nil)
 }
 
-func (s *WorkItem2Suite) TestWI2FailOnDelete() {
-	c := minimumRequiredCreatePayload()
-	c.Data.Attributes[workitem.SystemTitle] = "Title"
-	c.Data.Attributes[workitem.SystemState] = workitem.SystemStateNew
-	c.Data.Relationships.BaseType = newRelationBaseType(workitem.SystemBug)
-
-	_, createdWI := test.CreateWorkitemsCreated(s.T(), s.svc.Context, s.svc, s.workitemsCtrl, *c.Data.Relationships.Space.Data.ID, &c)
-	test.ShowWorkitemOK(s.T(), s.svc.Context, s.svc, s.workitemCtrl, *createdWI.Data.ID, nil, nil)
-	test.DeleteWorkitemMethodNotAllowed(s.T(), s.svc.Context, s.svc, s.workitemCtrl, *createdWI.Data.ID)
-}
-
 func (s *WorkItem2Suite) TestWI2CreateWithArea() {
 	fxt := tf.NewTestFixture(s.T(), s.DB,
 		tf.CreateWorkItemEnvironment(),
@@ -3392,3 +3381,25 @@ func (s *WorkItem2Suite) TestCreateAndUpdateWorkItemForEveryWIT() {
 		})
 	}
 }
+
+func (s *WorkItem2Suite) TestDeleteWorkitem() {
+	s.T().Run("ok", func(t *testing.T) {
+		fxt := tf.NewTestFixture(s.T(), s.DB, tf.WorkItems(1))
+		s.svc = testsupport.ServiceAsUser("TestUpdateWI2-Service", *fxt.Identities[0])
+		test.DeleteWorkitemOK(s.T(), s.svc.Context, s.svc, s.workitemCtrl, fxt.WorkItems[0].ID)
+	})
+	s.T().Run("unauthorized", func(t *testing.T) {
+		fxt := tf.NewTestFixture(s.T(), s.DB, tf.WorkItems(1), tf.Identities(2))
+		svcNotAuthorized := goa.New("TestDeleteWI2-Service")
+		workitemCtrlNotAuthorized := NewWorkitemController(svcNotAuthorized, s.GormDB, s.Configuration)
+		test.DeleteWorkitemUnauthorized(s.T(), svcNotAuthorized.Context, svcNotAuthorized, workitemCtrlNotAuthorized, fxt.WorkItems[0].ID)
+	})
+	s.T().Run("forbidden", func(t *testing.T) {
+		fxt := tf.NewTestFixture(s.T(), s.DB, tf.WorkItems(1), tf.Identities(2))
+		s.svc = testsupport.ServiceAsUser("TestUpdateWI2-Service", *fxt.Identities[1])
+		test.DeleteWorkitemForbidden(s.T(), s.svc.Context, s.svc, s.workitemCtrl, fxt.WorkItems[0].ID)
+	})
+	s.T().Run("workitem not found", func(t *testing.T) {
+		test.DeleteWorkitemNotFound(s.T(), s.svc.Context, s.svc, s.workitemCtrl, uuid.NewV4())
+	})
+}

design/workitems.go

@@ -152,9 +152,7 @@ var _ = a.Resource("workitem", func() {
 		a.Params(func() {
 			a.Param("wiID", d.UUID, "ID of the work item to delete")
 		})
-		a.Response(d.MethodNotAllowed)
 		a.Response(d.OK)
-		a.Response(d.BadRequest, JSONAPIErrors)
 		a.Response(d.InternalServerError, JSONAPIErrors)
 		a.Response(d.NotFound, JSONAPIErrors)
 		a.Response(d.Unauthorized, JSONAPIErrors)