more tests and rbac fix

coder · f0ssel · Jul 16, 2024 · Jul 9, 2024 · Jul 9, 2024 · Jul 11, 2024
commit 5ce5549153b97a133d8fb63834e5b99ed73c5169
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -918,7 +918,7 @@ func New(options *Options) *API {
 				r.Route("/provisionerkeys", func(r chi.Router) {
 					r.Get("/", api.provisionerKeys)
 					r.Post("/", api.postProvisionerKey)
-					r.Route("/{provisionerKey}", func(r chi.Router) {
+					r.Route("/{provisionerkey}", func(r chi.Router) {
 						r.Use(
 							httpmw.ExtractProvisionerKeyParam(options.Database),
 						)

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -1075,10 +1075,7 @@ func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.Del
 }
 
 func (q *querier) DeleteProvisionerKey(ctx context.Context, id uuid.UUID) error {
-	if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceProvisionerKeys); err != nil {
-		return err
-	}
-	return q.db.DeleteProvisionerKey(ctx, id)
+	return deleteQ(q.log, q.auth, q.db.GetProvisionerKeyByID, q.db.DeleteProvisionerKey)(ctx, id)
 }
 
 func (q *querier) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error {
@@ -1678,11 +1675,12 @@ func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt
 	return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt)
 }
 
+func (q *querier) GetProvisionerKeyByID(ctx context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
+	return fetch(q.log, q.auth, q.db.GetProvisionerKeyByID)(ctx, id)
+}
+
 func (q *querier) GetProvisionerKeyByName(ctx context.Context, name database.GetProvisionerKeyByNameParams) (database.ProvisionerKey, error) {
-	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerKeys); err != nil {
-		return database.ProvisionerKey{}, err
-	}
-	return q.db.GetProvisionerKeyByName(ctx, name)
+	return fetch(q.log, q.auth, q.db.GetProvisionerKeyByName)(ctx, name)
 }
 
 func (q *querier) GetProvisionerLogsAfterID(ctx context.Context, arg database.GetProvisionerLogsAfterIDParams) ([]database.ProvisionerJobLog, error) {
@@ -2630,10 +2628,7 @@ func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.Ins
 }
 
 func (q *querier) InsertProvisionerKey(ctx context.Context, arg database.InsertProvisionerKeyParams) (database.ProvisionerKey, error) {
-	if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceProvisionerKeys); err != nil {
-		return database.ProvisionerKey{}, err
-	}
-	return q.db.InsertProvisionerKey(ctx, arg)
+	return insert(q.log, q.auth, rbac.ResourceProvisionerKeys.InOrg(arg.OrganizationID), q.db.InsertProvisionerKey)(ctx, arg)
 }
 
 func (q *querier) InsertReplica(ctx context.Context, arg database.InsertReplicaParams) (database.Replica, error) {
@@ -2865,10 +2860,7 @@ func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg datab
 }
 
 func (q *querier) ListProvisionerKeysByOrganization(ctx context.Context, organizationID uuid.UUID) ([]database.ListProvisionerKeysByOrganizationRow, error) {
-	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerKeys); err != nil {
-		return nil, err
-	}
-	return q.db.ListProvisionerKeysByOrganization(ctx, organizationID)
+	return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.ListProvisionerKeysByOrganization)(ctx, organizationID)
 }
 
 func (q *querier) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]database.WorkspaceAgentPortShare, error) {

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -3210,6 +3210,19 @@ func (q *FakeQuerier) GetProvisionerJobsCreatedAfter(_ context.Context, after ti
 	return jobs, nil
 }
 
+func (q *FakeQuerier) GetProvisionerKeyByID(ctx context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
+	q.mutex.RLock()
+	defer q.mutex.RUnlock()
+
+	for _, key := range q.provisionerKeys {
+		if key.ID == id {
+			return key, nil
+		}
+	}
+
+	return database.ProvisionerKey{}, sql.ErrNoRows
+}
+
 func (q *FakeQuerier) GetProvisionerKeyByName(ctx context.Context, arg database.GetProvisionerKeyByNameParams) (database.ProvisionerKey, error) {
 	q.mutex.RLock()
 	defer q.mutex.RUnlock()

diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -218,6 +218,12 @@ func (p ProvisionerKey) RBACObject() rbac.Object {
 		InOrg(p.OrganizationID)
 }
 
+func (p ListProvisionerKeysByOrganizationRow) RBACObject() rbac.Object {
+	return rbac.ResourceProvisionerKeys.
+		WithID(p.ID).
+		InOrg(p.OrganizationID)
+}
+
 func (w WorkspaceProxy) RBACObject() rbac.Object {
 	return rbac.ResourceWorkspaceProxy.
 		WithID(w.ID)

diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/provisionerkeys.sql b/coderd/database/queries/provisionerkeys.sql
@@ -10,6 +10,14 @@ INSERT INTO
 VALUES
 	($1, $2, $3, $4, $5) RETURNING *;
 
+-- name: GetProvisionerKeyByID :one
+SELECT
+    *
+FROM
+    provisioner_keys
+WHERE
+    id = $1;
+
 -- name: GetProvisionerKeyByName :one
 SELECT
     *

diff --git a/coderd/httpmw/provisionerkey.go b/coderd/httpmw/provisionerkey.go
@@ -15,7 +15,7 @@ type provisionerKeyParamContextKey struct{}
 
 // ProvisionerKeyParam returns the user from the ExtractProvisionerKeyParam handler.
 func ProvisionerKeyParam(r *http.Request) database.ProvisionerKey {
-	user, ok := r.Context().Value(userParamContextKey{}).(database.ProvisionerKey)
+	user, ok := r.Context().Value(provisionerKeyParamContextKey{}).(database.ProvisionerKey)
 	if !ok {
 		panic("developer error: provisioner key parameter middleware not provided")
 	}
@@ -30,10 +30,10 @@ func ExtractProvisionerKeyParam(db database.Store) func(http.Handler) http.Handl
 			ctx := r.Context()
 			organization := OrganizationParam(r)
 
-			provisionerKeyQuery := chi.URLParam(r, "provisionerKey")
+			provisionerKeyQuery := chi.URLParam(r, "provisionerkey")
 			if provisionerKeyQuery == "" {
 				httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-					Message: "\"provisionerKey\" must be provided.",
+					Message: "\"provisionerkey\" must be provided.",
 				})
 				return
 			}

diff --git a/coderd/provisionerkeys_test.go b/coderd/provisionerkeys_test.go
@@ -15,7 +15,7 @@ import (
 func TestProvisionerKeys(t *testing.T) {
 	t.Parallel()
 
-	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong*10)
 	t.Cleanup(cancel)
 	client := coderdtest.New(t, nil)
 	owner := coderdtest.CreateFirstUser(t, client)
@@ -70,7 +70,25 @@ func TestProvisionerKeys(t *testing.T) {
 	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
 		Name: "key",
 	})
-	require.Error(t, err, "org admin create provisioner key")
+	require.ErrorContains(t, err, "already exists")
+
+	// key name cannot have special characters
+	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
+		Name: "key with spaces",
+	})
+	require.ErrorContains(t, err, "org admin create provisioner key")
+
+	// key name cannot be too long
+	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
+		Name: "key with spaces",
+	})
+	require.ErrorContains(t, err, "less than 64 characters")
+
+	// key name cannot be empty
+	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
+		Name: "",
+	})
+	require.ErrorContains(t, err, "cannot be empty")
 
 	// org admin can list provisioner keys
 	keys, err = orgAdmin.ListProvisionerKeys(ctx, owner.OrganizationID)
@@ -83,5 +101,5 @@ func TestProvisionerKeys(t *testing.T) {
 
 	// org admin cannot delete a provisioner key that doesn't exist
 	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, "key")
-	require.Error(t, err, "org admin delete provisioner key")
+	require.ErrorContains(t, err, "Resource not found")
 }
diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go
@@ -310,7 +310,7 @@ func (c *Client) ListProvisionerKeys(ctx context.Context, organizationID uuid.UU
 	}
 	defer res.Body.Close()
 
-	if res.StatusCode != http.StatusCreated {
+	if res.StatusCode != http.StatusOK {
 		return nil, ReadBodyAsError(res)
 	}
 	var resp []ProvisionerKey

diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts