feat: RBAC provisionerdaemons and parameters (#1755)

* chore: Remove org_id from provisionerdaemons

coderd/coderd.go

@@ -131,6 +131,13 @@ func New(options *Options) *API {
 			r.Get("/{hash}", api.fileByHash)
 			r.Post("/", api.postFile)
 		})
+		r.Route("/provisionerdaemons", func(r chi.Router) {
+			r.Use(
+				apiKeyMiddleware,
+				authRolesMiddleware,
+			)
+			r.Get("/", api.provisionerDaemons)
+		})
 		r.Route("/organizations/{organization}", func(r chi.Router) {
 			r.Use(
 				apiKeyMiddleware,
@@ -138,7 +145,6 @@ func New(options *Options) *API {
 				authRolesMiddleware,
 			)
 			r.Get("/", api.organization)
-			r.Get("/provisionerdaemons", api.provisionerDaemonsByOrganization)
 			r.Post("/templateversions", api.postTemplateVersionsByOrganization)
 			r.Route("/templates", func(r chi.Router) {
 				r.Post("/", api.postTemplateByOrganization)
@@ -166,7 +172,7 @@ func New(options *Options) *API {
 			})
 		})
 		r.Route("/parameters/{scope}/{id}", func(r chi.Router) {
-			r.Use(apiKeyMiddleware)
+			r.Use(apiKeyMiddleware, authRolesMiddleware)
 			r.Post("/", api.postParameter)
 			r.Get("/", api.parameters)
 			r.Route("/{name}", func(r chi.Router) {

coderd/coderd_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/go-chi/chi/v5"
 	"github.com/stretchr/testify/assert"
@@ -45,9 +46,29 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 		IncludeProvisionerD: true,
 	})
 	admin := coderdtest.CreateFirstUser(t, client)
-	organization, err := client.Organization(context.Background(), admin.OrganizationID)
+	// The provisioner will call to coderd and register itself. This is async,
+	// so we wait for it to occur.
+	require.Eventually(t, func() bool {
+		provisionerds, err := client.ProvisionerDaemons(ctx)
+		require.NoError(t, err)
+		return len(provisionerds) > 0
+	}, time.Second*10, time.Second)
+
+	provisionerds, err := client.ProvisionerDaemons(ctx)
+	require.NoError(t, err, "fetch provisioners")
+	require.Len(t, provisionerds, 1)
+
+	organization, err := client.Organization(ctx, admin.OrganizationID)
 	require.NoError(t, err, "fetch org")
 
+	organizationParam, err := client.CreateParameter(ctx, codersdk.ParameterOrganization, organization.ID, codersdk.CreateParameterRequest{
+		Name:              "test-param",
+		SourceValue:       "hello world",
+		SourceScheme:      codersdk.ParameterSourceSchemeData,
+		DestinationScheme: codersdk.ParameterDestinationSchemeProvisionerVariable,
+	})
+	require.NoError(t, err, "create org param")
+
 	// Setup some data in the database.
 	version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, &echo.Responses{
 		Parse: echo.ParseComplete,
@@ -118,18 +139,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 		"GET:/api/v2/workspaceagents/{workspaceagent}/turn":       {NoAuthorize: true},
 
 		// TODO: @emyrk these need to be fixed by adding authorize calls
-		"GET:/api/v2/organizations/{organization}/provisionerdaemons":       {NoAuthorize: true},
-		"GET:/api/v2/organizations/{organization}/templates/{templatename}": {NoAuthorize: true},
-		"POST:/api/v2/organizations/{organization}/templateversions":        {NoAuthorize: true},
-		"POST:/api/v2/organizations/{organization}/workspaces":              {NoAuthorize: true},
-
-		"POST:/api/v2/parameters/{scope}/{id}":          {NoAuthorize: true},
-		"GET:/api/v2/parameters/{scope}/{id}":           {NoAuthorize: true},
-		"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {NoAuthorize: true},
-
-		"POST:/api/v2/users/{user}/organizations": {NoAuthorize: true},
-
-		"GET:/api/v2/workspaces/{workspace}/watch": {NoAuthorize: true},
+		"POST:/api/v2/organizations/{organization}/workspaces":       {NoAuthorize: true},
+		"POST:/api/v2/users/{user}/organizations":                    {NoAuthorize: true},
+		"GET:/api/v2/workspaces/{workspace}/watch":                   {NoAuthorize: true},
+		"POST:/api/v2/organizations/{organization}/templateversions": {NoAuthorize: true},
 
 		// These endpoints have more assertions. This is good, add more endpoints to assert if you can!
 		"GET:/api/v2/organizations/{organization}":                   {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)},
@@ -251,6 +264,27 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
 		},
+		"GET:/api/v2/provisionerdaemons": {
+			StatusCode:   http.StatusOK,
+			AssertObject: rbac.ResourceProvisionerDaemon.WithID(provisionerds[0].ID.String()),
+		},
+
+		"POST:/api/v2/parameters/{scope}/{id}": {
+			AssertAction: rbac.ActionUpdate,
+			AssertObject: rbac.ResourceOrganization.WithID(organization.ID.String()),
+		},
+		"GET:/api/v2/parameters/{scope}/{id}": {
+			AssertAction: rbac.ActionRead,
+			AssertObject: rbac.ResourceOrganization.WithID(organization.ID.String()),
+		},
+		"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {
+			AssertAction: rbac.ActionUpdate,
+			AssertObject: rbac.ResourceOrganization.WithID(organization.ID.String()),
+		},
+		"GET:/api/v2/organizations/{organization}/templates/{templatename}": {
+			AssertAction: rbac.ActionRead,
+			AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
+		},
 
 		// These endpoints need payloads to get to the auth part. Payloads will be required
 		"PUT:/api/v2/users/{user}/roles":                                {StatusCode: http.StatusBadRequest, NoAuthorize: true},
@@ -292,6 +326,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 			route = strings.ReplaceAll(route, "{hash}", file.Hash)
 			route = strings.ReplaceAll(route, "{workspaceresource}", workspaceResources[0].ID.String())
 			route = strings.ReplaceAll(route, "{templateversion}", version.ID.String())
+			route = strings.ReplaceAll(route, "{templatename}", template.Name)
+			// Only checking org scoped params here
+			route = strings.ReplaceAll(route, "{scope}", string(organizationParam.Scope))
+			route = strings.ReplaceAll(route, "{id}", organizationParam.ScopeID.String())
 
 			resp, err := client.Request(context.Background(), method, route, nil)
 			require.NoError(t, err, "do req")

coderd/database/databasefake/databasefake.go

@@ -1283,11 +1283,10 @@ func (q *fakeQuerier) InsertProvisionerDaemon(_ context.Context, arg database.In
 	defer q.mutex.Unlock()
 
 	daemon := database.ProvisionerDaemon{
-		ID:             arg.ID,
-		CreatedAt:      arg.CreatedAt,
-		OrganizationID: arg.OrganizationID,
-		Name:           arg.Name,
-		Provisioners:   arg.Provisioners,
+		ID:           arg.ID,
+		CreatedAt:    arg.CreatedAt,
+		Name:         arg.Name,
+		Provisioners: arg.Provisioners,
 	}
 	q.provisionerDaemons = append(q.provisionerDaemons, daemon)
 	return daemon, nil

coderd/database/dump.sql

@@ -194,7 +194,6 @@ CREATE TABLE provisioner_daemons (
     id uuid NOT NULL,
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone,
-    organization_id uuid,
     name character varying(64) NOT NULL,
     provisioners provisioner_type[] NOT NULL
 );

coderd/database/migrations/000014_provisioner_daemons_organization_id.down.sql

@@ -0,0 +1 @@
+ALTER TABLE provisioner_daemons ADD COLUMN organization_id uuid;

coderd/database/migrations/000014_provisioner_daemons_organization_id.up.sql

@@ -0,0 +1 @@
+ALTER TABLE provisioner_daemons DROP COLUMN organization_id;

coderd/database/modelmethods.go

@@ -22,3 +22,7 @@ func (m OrganizationMember) RBACObject() rbac.Object {
 func (o Organization) RBACObject() rbac.Object {
 	return rbac.ResourceOrganization.InOrg(o.ID).WithID(o.ID.String())
 }
+
+func (d ProvisionerDaemon) RBACObject() rbac.Object {
+	return rbac.ResourceProvisionerDaemon.WithID(d.ID.String())
+}

coderd/database/models.go

@@ -391,12 +391,11 @@ type ParameterValue struct {
 }
 
 type ProvisionerDaemon struct {
-	ID             uuid.UUID         `db:"id" json:"id"`
-	CreatedAt      time.Time         `db:"created_at" json:"created_at"`
-	UpdatedAt      sql.NullTime      `db:"updated_at" json:"updated_at"`
-	OrganizationID uuid.NullUUID     `db:"organization_id" json:"organization_id"`
-	Name           string            `db:"name" json:"name"`
-	Provisioners   []ProvisionerType `db:"provisioners" json:"provisioners"`
+	ID           uuid.UUID         `db:"id" json:"id"`
+	CreatedAt    time.Time         `db:"created_at" json:"created_at"`
+	UpdatedAt    sql.NullTime      `db:"updated_at" json:"updated_at"`
+	Name         string            `db:"name" json:"name"`
+	Provisioners []ProvisionerType `db:"provisioners" json:"provisioners"`
 }
 
 type ProvisionerJob struct {

coderd/database/queries.sql.go

@@ -1076,7 +1076,7 @@ func (q *sqlQuerier) InsertParameterValue(ctx context.Context, arg InsertParamet
 
 const getProvisionerDaemonByID = `-- name: GetProvisionerDaemonByID :one
 SELECT
-	id, created_at, updated_at, organization_id, name, provisioners
+	id, created_at, updated_at, name, provisioners
 FROM
 	provisioner_daemons
 WHERE
@@ -1090,7 +1090,6 @@ func (q *sqlQuerier) GetProvisionerDaemonByID(ctx context.Context, id uuid.UUID)
 		&i.ID,
 		&i.CreatedAt,
 		&i.UpdatedAt,
-		&i.OrganizationID,
 		&i.Name,
 		pq.Array(&i.Provisioners),
 	)
@@ -1099,7 +1098,7 @@ func (q *sqlQuerier) GetProvisionerDaemonByID(ctx context.Context, id uuid.UUID)
 
 const getProvisionerDaemons = `-- name: GetProvisionerDaemons :many
 SELECT
-	id, created_at, updated_at, organization_id, name, provisioners
+	id, created_at, updated_at, name, provisioners
 FROM
 	provisioner_daemons
 `
@@ -1117,7 +1116,6 @@ func (q *sqlQuerier) GetProvisionerDaemons(ctx context.Context) ([]ProvisionerDa
 			&i.ID,
 			&i.CreatedAt,
 			&i.UpdatedAt,
-			&i.OrganizationID,
 			&i.Name,
 			pq.Array(&i.Provisioners),
 		); err != nil {
@@ -1139,27 +1137,24 @@ INSERT INTO
 	provisioner_daemons (
 		id,
 		created_at,
-		organization_id,
 		"name",
 		provisioners
 	)
 VALUES
-	($1, $2, $3, $4, $5) RETURNING id, created_at, updated_at, organization_id, name, provisioners
+	($1, $2, $3, $4) RETURNING id, created_at, updated_at, name, provisioners
 `
 
 type InsertProvisionerDaemonParams struct {
-	ID             uuid.UUID         `db:"id" json:"id"`
-	CreatedAt      time.Time         `db:"created_at" json:"created_at"`
-	OrganizationID uuid.NullUUID     `db:"organization_id" json:"organization_id"`
-	Name           string            `db:"name" json:"name"`
-	Provisioners   []ProvisionerType `db:"provisioners" json:"provisioners"`
+	ID           uuid.UUID         `db:"id" json:"id"`
+	CreatedAt    time.Time         `db:"created_at" json:"created_at"`
+	Name         string            `db:"name" json:"name"`
+	Provisioners []ProvisionerType `db:"provisioners" json:"provisioners"`
 }
 
 func (q *sqlQuerier) InsertProvisionerDaemon(ctx context.Context, arg InsertProvisionerDaemonParams) (ProvisionerDaemon, error) {
 	row := q.db.QueryRowContext(ctx, insertProvisionerDaemon,
 		arg.ID,
 		arg.CreatedAt,
-		arg.OrganizationID,
 		arg.Name,
 		pq.Array(arg.Provisioners),
 	)
@@ -1168,7 +1163,6 @@ func (q *sqlQuerier) InsertProvisionerDaemon(ctx context.Context, arg InsertProv
 		&i.ID,
 		&i.CreatedAt,
 		&i.UpdatedAt,
-		&i.OrganizationID,
 		&i.Name,
 		pq.Array(&i.Provisioners),
 	)

coderd/database/queries/provisionerdaemons.sql

@@ -17,12 +17,11 @@ INSERT INTO
 	provisioner_daemons (
 		id,
 		created_at,
-		organization_id,
 		"name",
 		provisioners
 	)
 VALUES
-	($1, $2, $3, $4, $5) RETURNING *;
+	($1, $2, $3, $4) RETURNING *;
 
 -- name: UpdateProvisionerDaemonByID :exec
 UPDATE

coderd/parameters.go

@@ -13,18 +13,27 @@ import (
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/coderd/parameter"
+	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/codersdk"
 )
 
 func (api *API) postParameter(rw http.ResponseWriter, r *http.Request) {
-	var createRequest codersdk.CreateParameterRequest
-	if !httpapi.Read(rw, r, &createRequest) {
-		return
-	}
 	scope, scopeID, valid := readScopeAndID(rw, r)
 	if !valid {
 		return
 	}
+	obj, ok := api.parameterRBACResource(rw, r, scope, scopeID)
+	if !ok {
+		return
+	}
+	if !api.Authorize(rw, r, rbac.ActionUpdate, obj) {
+		return
+	}
+
+	var createRequest codersdk.CreateParameterRequest
+	if !httpapi.Read(rw, r, &createRequest) {
+		return
+	}
 	_, err := api.Database.GetParameterValueByScopeAndName(r.Context(), database.GetParameterValueByScopeAndNameParams{
 		Scope:   scope,
 		ScopeID: scopeID,
@@ -42,6 +51,7 @@ func (api *API) postParameter(rw http.ResponseWriter, r *http.Request) {
 		})
 		return
 	}
+
 	parameterValue, err := api.Database.InsertParameterValue(r.Context(), database.InsertParameterValueParams{
 		ID:                uuid.New(),
 		Name:              createRequest.Name,
@@ -68,6 +78,15 @@ func (api *API) parameters(rw http.ResponseWriter, r *http.Request) {
 	if !valid {
 		return
 	}
+	obj, ok := api.parameterRBACResource(rw, r, scope, scopeID)
+	if !ok {
+		return
+	}
+
+	if !api.Authorize(rw, r, rbac.ActionRead, obj) {
+		return
+	}
+
 	parameterValues, err := api.Database.GetParameterValuesByScope(r.Context(), database.GetParameterValuesByScopeParams{
 		Scope:   scope,
 		ScopeID: scopeID,
@@ -94,6 +113,15 @@ func (api *API) deleteParameter(rw http.ResponseWriter, r *http.Request) {
 	if !valid {
 		return
 	}
+	obj, ok := api.parameterRBACResource(rw, r, scope, scopeID)
+	if !ok {
+		return
+	}
+	// A delete param is still updating the underlying resource for the scope.
+	if !api.Authorize(rw, r, rbac.ActionUpdate, obj) {
+		return
+	}
+
 	name := chi.URLParam(r, "name")
 	parameterValue, err := api.Database.GetParameterValueByScopeAndName(r.Context(), database.GetParameterValueByScopeAndNameParams{
 		Scope:   scope,
@@ -168,6 +196,53 @@ func convertParameterValue(parameterValue database.ParameterValue) codersdk.Para
 	}
 }
 
+// parameterRBACResource returns the RBAC resource a parameter scope and scope
+// ID is trying to update. For RBAC purposes, adding a param to a resource
+// is equivalent to updating/reading the associated resource.
+// This means "parameters" are not a new resource, but an extension of existing
+// ones.
+func (api *API) parameterRBACResource(rw http.ResponseWriter, r *http.Request, scope database.ParameterScope, scopeID uuid.UUID) (rbac.Objecter, bool) {
+	ctx := r.Context()
+	var resource rbac.Objecter
+	var err error
+	switch scope {
+	case database.ParameterScopeWorkspace:
+		resource, err = api.Database.GetWorkspaceByID(ctx, scopeID)
+	case database.ParameterScopeTemplate:
+		resource, err = api.Database.GetTemplateByID(ctx, scopeID)
+	case database.ParameterScopeOrganization:
+		resource, err = api.Database.GetOrganizationByID(ctx, scopeID)
+	case database.ParameterScopeUser:
+		user, userErr := api.Database.GetUserByID(ctx, scopeID)
+		err = userErr
+		if err != nil {
+			// Use the userdata resource instead of the user. This way users
+			// can add user scoped params.
+			resource = rbac.ResourceUserData.WithID(user.ID.String()).WithOwner(user.ID.String())
+		}
+	case database.ParameterScopeImportJob:
+		// This scope does not make sense from this api.
+		// ImportJob params are created with the job, and the job id cannot
+		// be predicted.
+		err = xerrors.Errorf("ImportJob scope not supported")
+	default:
+		err = xerrors.Errorf("scope %q unsupported", scope)
+	}
+
+	// Write error payload to rw if we cannot find the resource for the scope
+	if err != nil {
+		if xerrors.Is(err, sql.ErrNoRows) {
+			httpapi.Forbidden(rw)
+		} else {
+			httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
+				Message: fmt.Sprintf("param scope resource: %s", err.Error()),
+			})
+		}
+		return nil, false
+	}
+	return resource, true
+}
+
 func readScopeAndID(rw http.ResponseWriter, r *http.Request) (database.ParameterScope, uuid.UUID, bool) {
 	var scope database.ParameterScope
 	switch chi.URLParam(r, "scope") {

coderd/provisionerdaemons.go

@@ -25,12 +25,13 @@ import (
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/coderd/parameter"
+	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/provisionerd/proto"
 	"github.com/coder/coder/provisionersdk"
 	sdkproto "github.com/coder/coder/provisionersdk/proto"
 )
 
-func (api *API) provisionerDaemonsByOrganization(rw http.ResponseWriter, r *http.Request) {
+func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) {
 	daemons, err := api.Database.GetProvisionerDaemons(r.Context())
 	if errors.Is(err, sql.ErrNoRows) {
 		err = nil
@@ -44,6 +45,8 @@ func (api *API) provisionerDaemonsByOrganization(rw http.ResponseWriter, r *http
 	if daemons == nil {
 		daemons = []database.ProvisionerDaemon{}
 	}
+	daemons = AuthorizeFilter(api, r, rbac.ActionRead, daemons)
+
 	httpapi.Write(rw, http.StatusOK, daemons)
 }
 

coderd/provisionerdaemons_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 
 	"github.com/coder/coder/coderd/coderdtest"
@@ -51,15 +50,15 @@ func TestProvisionerDaemonsByOrganization(t *testing.T) {
 	t.Run("NoAuth", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
-		_, err := client.ProvisionerDaemonsByOrganization(context.Background(), uuid.New())
+		_, err := client.ProvisionerDaemons(context.Background())
 		require.Error(t, err)
 	})
 
 	t.Run("Get", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
-		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.ProvisionerDaemonsByOrganization(context.Background(), user.OrganizationID)
+		_ = coderdtest.CreateFirstUser(t, client)
+		_, err := client.ProvisionerDaemons(context.Background())
 		require.NoError(t, err)
 	})
 }

coderd/rbac/builtin.go

@@ -68,6 +68,8 @@ var (
 					// All users can read all other users and know they exist.
 					ResourceUser:           {ActionRead},
 					ResourceRoleAssignment: {ActionRead},
+					// All users can see the provisioner daemons.
+					ResourceProvisionerDaemon: {ActionRead},
 				}),
 				User: permissions(map[Object][]Action{
 					ResourceWildcard: {WildcardSymbol},

coderd/rbac/object.go

@@ -34,6 +34,10 @@ var (
 		Type: "file",
 	}
 
+	ResourceProvisionerDaemon = Object{
+		Type: "provisioner_daemon",
+	}
+
 	// ResourceOrganization CRUD. Has an org owner on all but 'create'.
 	//	create/delete = make or delete organizations
 	// 	read = view org information (Can add user owner for read)