Allow master tenant to create cross-owner batch actions

Author: jakub-id

broker/oapi/open-api.yaml

@@ -2516,7 +2516,7 @@ paths:
                 $ref: '#/components/schemas/Error'
     post:
       summary: Create a new batch action
-      description: Creates a batch action for one owner. Master access must provide `symbol`; Okapi access defaults to the tenant's primary symbol when omitted.
+      description: Creates a batch action. Master access without `symbol` creates an unrestricted action; Okapi access without `symbol` defaults to the tenant's primary symbol.
       tags:
         - scheduler-api
       parameters:

broker/scheduler/api/api_handler.go

@@ -123,7 +123,7 @@ func (h SchedulerApiHandler) PostBatchActions(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	owner, ok := h.resolveConcreteOwner(ctx, w, r, params.Symbol)
+	owner, ok := h.resolveBatchActionOwner(ctx, w, r, params.Symbol)
 	if !ok {
 		return
 	}
@@ -325,8 +325,9 @@ func (h SchedulerApiHandler) resolveOwnerScope(ctx common.ExtendedContext, w htt
 	return owners, true
 }
 
-// resolveConcreteOwner resolves the single owner required when creating a task.
-func (h SchedulerApiHandler) resolveConcreteOwner(ctx common.ExtendedContext, w http.ResponseWriter, r *http.Request, symbol *string) (string, bool) {
+// resolveBatchActionOwner resolves the owner for a new batch action. An empty
+// owner represents unrestricted master access.
+func (h SchedulerApiHandler) resolveBatchActionOwner(ctx common.ExtendedContext, w http.ResponseWriter, r *http.Request, symbol *string) (string, bool) {
 	t, err := h.tenantResolver.Resolve(ctx, r, symbol)
 	if err != nil {
 		brokerapi.AddBadRequestError(ctx, w, err)
@@ -337,10 +338,6 @@ func (h SchedulerApiHandler) resolveConcreteOwner(ctx common.ExtendedContext, w
 		brokerapi.AddBadRequestError(ctx, w, err)
 		return "", false
 	}
-	if owner == "" {
-		brokerapi.AddBadRequestError(ctx, w, errors.New("symbol must be specified when creating a batch action with master access"))
-		return "", false
-	}
 	return owner, true
 }
 

broker/scheduler/api/api_handler_test.go

@@ -409,17 +409,48 @@ func TestPostBatchActions_ActionParamsPersisted(t *testing.T) {
 	repo.AssertExpectations(t)
 }
 
-func TestPostBatchActions_MasterWithoutSymbolReturnsBadRequest(t *testing.T) {
+func TestPostBatchActions_MasterWithoutSymbolCreatesUnrestrictedAction(t *testing.T) {
 	repo := new(MockSchedRepo)
+	repo.On("SaveScheduledTask", mock.MatchedBy(func(p sched_db.SaveScheduledTaskParams) bool {
+		return p.Owner == "" &&
+			p.ActionData.BatchActionData != nil &&
+			p.ActionData.BatchActionData.Owner == ""
+	})).Return(saveScheduledTaskReturn, nil)
+
 	h := newHandler(repo)
-	req := newReq(http.MethodPost, `{"actionName":"email-pullslips","batchQuery":"title=test","schedule":"`+validRrule+`"}`)
+	req := newReq(http.MethodPost, `{"actionName":"request-aging","batchQuery":"title=test","schedule":"`+validRrule+`","actionParams":{"interval":"24h"}}`)
 	rr := httptest.NewRecorder()
 
 	h.PostBatchActions(rr, req, schedoapi.PostBatchActionsParams{})
 
-	assert.Equal(t, http.StatusBadRequest, rr.Code)
-	assert.Contains(t, rr.Body.String(), "symbol must be specified")
-	repo.AssertNotCalled(t, "SaveScheduledTask", mock.Anything)
+	assert.Equal(t, http.StatusCreated, rr.Code)
+	var resp schedoapi.BatchAction
+	assert.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
+	assert.Equal(t, schedoapi.RequestAging, resp.ActionName)
+	repo.AssertExpectations(t)
+}
+
+func TestPostBatchActions_OkapiWithoutSymbolUsesPrimaryMappedOwner(t *testing.T) {
+	const mappedOwner = "ISIL:DK-DIKU"
+	repo := new(MockSchedRepo)
+	repo.On("SaveScheduledTask", mock.MatchedBy(func(p sched_db.SaveScheduledTaskParams) bool {
+		return p.Owner == mappedOwner &&
+			p.ActionData.BatchActionData != nil &&
+			p.ActionData.BatchActionData.Owner == mappedOwner
+	})).Return(saveScheduledTaskReturn, nil)
+	resolver := tenant.NewResolver().
+		WithTenantToSymbol("ISIL:DK-{tenant}").
+		WithIllRepo(new(testmocks.MockIllRepositorySuccess))
+	h := NewSchedulerApiHandler(10, repo, nil, resolver)
+	req := httptest.NewRequest(http.MethodPost, "/broker/batch_actions",
+		strings.NewReader(`{"actionName":"request-aging","batchQuery":"title=test","schedule":"`+validRrule+`","actionParams":{"interval":"24h"}}`))
+	req.Header.Set(tenant.OkapiTenantHeader, "diku")
+	rr := httptest.NewRecorder()
+
+	h.PostBatchActions(rr, req, schedoapi.PostBatchActionsParams{})
+
+	assert.Equal(t, http.StatusCreated, rr.Code)
+	repo.AssertExpectations(t)
 }
 
 func TestPostBatchActions_MissingBody(t *testing.T) {