[scheduler]: ACLs phase 1 - per Job ACL specification and enforcement.

scheduler/appengine/engine/engine.go

Index: scheduler/appengine/engine/engine.go
diff --git a/scheduler/appengine/engine/engine.go b/scheduler/appengine/engine/engine.go
index 57799b38eda49460691045f44521bc2fdc68d4d2..577bb9b2480a32693154ee42deb630fd024932c9 100644
--- a/scheduler/appengine/engine/engine.go
+++ b/scheduler/appengine/engine/engine.go
@@ -48,6 +48,7 @@ import (
 	"github.com/luci/luci-go/server/auth/signing"
 	"github.com/luci/luci-go/server/tokens"
 
+	"github.com/luci/luci-go/scheduler/appengine/acl"
 	"github.com/luci/luci-go/scheduler/appengine/catalog"
 	"github.com/luci/luci-go/scheduler/appengine/schedule"
 	"github.com/luci/luci-go/scheduler/appengine/task"
@@ -65,25 +66,28 @@ var (
 type Engine interface {
 	// GetAllProjects returns a list of all projects that have at least one
 	// enabled scheduler job.
+	// ACLs are NOT enforced; internal scheduler use only.
 	GetAllProjects(c context.Context) ([]string, error)
 
-	// GetAllJobs returns a list of all enabled scheduler jobs in no particular
-	// order.
-	GetAllJobs(c context.Context) ([]*Job, error)
+	// GetAllJobsRA returns a list of all enabled scheduler jobs to which
+	// current identity has READER access in no particular order.
+	GetAllJobsRA(c context.Context) ([]*Job, error)

[tandrii(chromium), 2017/08/02 20:23:46]

RA here and below stands for READER Access. I wish I can get rid of that prefix,
but GetAllProjects will be an outlier then :( 

 
-	// GetProjectJobs returns a list of enabled scheduler jobs of some project
-	// in no particular order.
-	GetProjectJobs(c context.Context, projectID string) ([]*Job, error)
+	// GetProjectJobsRA returns a list of enabled scheduler jobs to which curreent
+	// identity has READER access of some project in no particular order.
+	GetProjectJobsRA(c context.Context, projectID string) ([]*Job, error)
 
-	// GetJob returns single scheduler job given its full ID or nil if no such
-	// job.
-	GetJob(c context.Context, jobID string) (*Job, error)
+	// GetJobRA returns single scheduler job given its full ID if and only if
+	// current identity has READER access to it. Returns nil otherwise.
+	GetJobRA(c context.Context, jobID string) (*Job, error)
 
 	// ListInvocations returns invocations of a job, most recent first.
 	// Returns fetched invocations and cursor string if there's more.
+	// TODO(tandrii): enforce ACLs.
 	ListInvocations(c context.Context, jobID string, pageSize int, cursor string) ([]*Invocation, string, error)

[tandrii(chromium), 2017/08/02 20:23:46]

this and two methods below will fetch JobId concurrently (or sequentially for
...byNonce) to ensure ACLs. Internal engine will call lowercase versions. SGTM?

[Vadim Sh., 2017/08/02 21:14:13]

Yes.

 
 	// GetInvocation returns single invocation of some job given its ID.
+	// TODO(tandrii): enforce ACLs.
 	GetInvocation(c context.Context, jobID string, invID int64) (*Invocation, error)
 
 	// GetInvocationsByNonce returns a list of Invocations with given nonce.
@@ -91,6 +95,7 @@ type Engine interface {
 	// Invocation nonce is a random number that identifies an intent to start
 	// an invocation. Normally one nonce corresponds to one Invocation entity,
 	// but there can be more if job fails to start with a transient error.
+	// TODO(tandrii): enforce ACLs.
 	GetInvocationsByNonce(c context.Context, invNonce int64) ([]*Invocation, error)
 
 	// UpdateProjectJobs adds new, removes old and updates existing jobs.
@@ -269,6 +274,9 @@ type Job struct {
 	// of the engine. See Catalog.UnmarshalTask().
 	Task []byte `gae:",noindex"`
 
+	// ACLs are the latest ACLs applied to Job and all its invocations.
+	Acls acl.GrantsByRole `gae:",noindex"`
+
 	// State is the job's state machine state, see StateMachine.
 	State JobState
 }
@@ -328,6 +336,7 @@ func (e *Job) matches(def catalog.Definition) bool {
 	return e.JobID == def.JobID &&
 		e.Flavor == def.Flavor &&
 		e.Schedule == def.Schedule &&
+		e.Acls.Equal(&def.Acls) &&
 		bytes.Equal(e.Task, def.Task)
 }
 
@@ -619,17 +628,17 @@ func (e *engineImpl) GetAllProjects(c context.Context) ([]string, error) {
 	return out, nil
 }
 
-func (e *engineImpl) GetAllJobs(c context.Context) ([]*Job, error) {
+func (e *engineImpl) GetAllJobsRA(c context.Context) ([]*Job, error) {
 	q := ds.NewQuery("Job").Eq("Enabled", true)
-	return e.queryEnabledJobs(c, q)
+	return e.queryEnabledJobsRA(c, q)
 }
 
-func (e *engineImpl) GetProjectJobs(c context.Context, projectID string) ([]*Job, error) {
+func (e *engineImpl) GetProjectJobsRA(c context.Context, projectID string) ([]*Job, error) {
 	q := ds.NewQuery("Job").Eq("Enabled", true).Eq("ProjectID", projectID)
-	return e.queryEnabledJobs(c, q)
+	return e.queryEnabledJobsRA(c, q)
 }
 
-func (e *engineImpl) queryEnabledJobs(c context.Context, q *ds.Query) ([]*Job, error) {
+func (e *engineImpl) queryEnabledJobsRA(c context.Context, q *ds.Query) ([]*Job, error) {
 	entities := []*Job{}
 	if err := ds.GetAll(c, q, &entities); err != nil {
 		return nil, transient.Tag.Apply(err)
@@ -637,17 +646,29 @@ func (e *engineImpl) queryEnabledJobs(c context.Context, q *ds.Query) ([]*Job, e
 	// Non-ancestor query used, need to recheck filters.
 	filtered := make([]*Job, 0, len(entities))
 	for _, job := range entities {
-		if job.Enabled {
+		if !job.Enabled {
+			continue
+		}
+		// TODO(tandrii): improve batch ACLs check here to take advantage of likely
+		// shared ACLs between most jobs of the same project.
+		if ok, err := job.Acls.IsReader(c); err != nil {
+			return nil, transient.Tag.Apply(err)
+		} else if ok {
 			filtered = append(filtered, job)
 		}
 	}
 	return filtered, nil
 }
 
-func (e *engineImpl) GetJob(c context.Context, jobID string) (*Job, error) {
+func (e *engineImpl) GetJobRA(c context.Context, jobID string) (*Job, error) {
 	job := &Job{JobID: jobID}
 	switch err := ds.Get(c, job); {
 	case err == nil:
+		if hasAccess, err := job.Acls.IsReader(c); err != nil {
+			return nil, transient.Tag.Apply(err)
+		} else if !hasAccess {
+			return nil, nil
+		}
 		return job, nil
 	case err == ds.ErrNoSuchEntity:
 		return nil, nil