DELETE license API endpoint (#3697)

* DELETE license API endpoint

Signed-off-by: Spike Curtis <spike@coder.com>

* Fix new lint stuff

Signed-off-by: Spike Curtis <spike@coder.com>

Signed-off-by: Spike Curtis <spike@coder.com>

coderd/coderdtest/authtest.go

@@ -2,6 +2,7 @@ package coderdtest
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
 	"strconv"
@@ -43,6 +44,7 @@ type AuthTester struct {
 	File                  codersdk.UploadResponse
 	TemplateVersionDryRun codersdk.ProvisionerJob
 	TemplateParam         codersdk.Parameter
+	URLParams             map[string]string
 }
 
 func NewAuthTester(ctx context.Context, t *testing.T, options *Options) *AuthTester {
@@ -86,7 +88,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, options *Options) *AuthTes
 							Id:   "something",
 							Auth: &proto.Agent_Token{},
 							Apps: []*proto.App{{
-								Name: "app",
+								Name: "testapp",
 								Url:  "http://localhost:3000",
 							}},
 						}},
@@ -116,6 +118,28 @@ func NewAuthTester(ctx context.Context, t *testing.T, options *Options) *AuthTes
 	})
 	require.NoError(t, err, "create template param")
 
+	urlParameters := map[string]string{
+		"{organization}":       admin.OrganizationID.String(),
+		"{user}":               admin.UserID.String(),
+		"{organizationname}":   organization.Name,
+		"{workspace}":          workspace.ID.String(),
+		"{workspacebuild}":     workspace.LatestBuild.ID.String(),
+		"{workspacename}":      workspace.Name,
+		"{workspacebuildname}": workspace.LatestBuild.Name,
+		"{workspaceagent}":     workspaceResources[0].Agents[0].ID.String(),
+		"{buildnumber}":        strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10),
+		"{template}":           template.ID.String(),
+		"{hash}":               file.Hash,
+		"{workspaceresource}":  workspaceResources[0].ID.String(),
+		"{workspaceapp}":       workspaceResources[0].Agents[0].Apps[0].Name,
+		"{templateversion}":    version.ID.String(),
+		"{jobID}":              templateVersionDryRun.ID.String(),
+		"{templatename}":       template.Name,
+		// Only checking template scoped params here
+		"parameters/{scope}/{id}": fmt.Sprintf("parameters/%s/%s",
+			string(templateParam.Scope), templateParam.ScopeID.String()),
+	}
+
 	return &AuthTester{
 		t:                     t,
 		api:                   api,
@@ -130,6 +154,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, options *Options) *AuthTes
 		File:                  file,
 		TemplateVersionDryRun: templateVersionDryRun,
 		TemplateParam:         templateParam,
+		URLParams:             urlParameters,
 	}
 }
 
@@ -153,13 +178,13 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 		"POST:/api/v2/csp/reports":      {NoAuthorize: true},
 		"GET:/api/v2/entitlements":      {NoAuthorize: true},
 
-		"GET:/%40{user}/{workspacename}/apps/{application}/*": {
-			AssertAction: rbac.ActionRead,
-			AssertObject: workspaceRBACObj,
+		"GET:/%40{user}/{workspacename}/apps/{workspaceapp}/*": {
+			AssertAction: rbac.ActionCreate,
+			AssertObject: workspaceExecObj,
 		},
-		"GET:/@{user}/{workspacename}/apps/{application}/*": {
-			AssertAction: rbac.ActionRead,
-			AssertObject: workspaceRBACObj,
+		"GET:/@{user}/{workspacename}/apps/{workspaceapp}/*": {
+			AssertAction: rbac.ActionCreate,
+			AssertObject: workspaceExecObj,
 		},
 
 		// Has it's own auth
@@ -188,7 +213,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 			AssertObject: rbac.ResourceWorkspace,
 			AssertAction: rbac.ActionRead,
 		},
-		"GET:/api/v2/users/me/workspace/{workspacename}/builds/{buildnumber}": {
+		"GET:/api/v2/users/{user}/workspace/{workspacename}/builds/{buildnumber}": {
 			AssertObject: rbac.ResourceWorkspace,
 			AssertAction: rbac.ActionRead,
 		},
@@ -216,7 +241,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 			AssertAction: rbac.ActionUpdate,
 			AssertObject: workspaceRBACObj,
 		},
-		"PUT:/api/v2/workspaces/{workspace}/autostop": {
+		"PUT:/api/v2/workspaces/{workspace}/ttl": {
 			AssertAction: rbac.ActionUpdate,
 			AssertObject: workspaceRBACObj,
 		},
@@ -275,7 +300,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 			AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
 		},
 		"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
-		"GET:/api/v2/files/{fileHash}": {
+		"GET:/api/v2/files/{hash}": {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceFile.WithOwner(a.Admin.UserID.String()),
 		},
@@ -320,19 +345,19 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
 		},
-		"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}": {
+		"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}": {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
 		},
-		"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/resources": {
+		"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/resources": {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
 		},
-		"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/logs": {
+		"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/logs": {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
 		},
-		"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/cancel": {
+		"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/cancel": {
 			AssertAction: rbac.ActionRead,
 			AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
 		},
@@ -366,10 +391,6 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
 			AssertAction: rbac.ActionRead,
 			AssertObject: workspaceRBACObj,
 		},
-		"POST:/api/v2/users/{user}/organizations": {
-			AssertAction: rbac.ActionCreate,
-			AssertObject: rbac.ResourceOrganization,
-		},
 		"GET:/api/v2/users": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceUser},
 
 		// These endpoints need payloads to get to the auth part. Payloads will be required
@@ -385,6 +406,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
 	// Always fail auth from this point forward
 	a.authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
 
+	routeMissing := make(map[string]bool)
 	for k, v := range assertRoute {
 		noTrailSlash := strings.TrimRight(k, "/")
 		if _, ok := assertRoute[noTrailSlash]; ok && noTrailSlash != k {
@@ -392,6 +414,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
 			a.t.FailNow()
 		}
 		assertRoute[noTrailSlash] = v
+		routeMissing[noTrailSlash] = true
 	}
 
 	for k, v := range skipRoutes {
@@ -422,32 +445,18 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
 			}
 			a.t.Run(name, func(t *testing.T) {
 				a.authorizer.reset()
-				routeAssertions, ok := assertRoute[strings.TrimRight(name, "/")]
+				routeKey := strings.TrimRight(name, "/")
+				routeAssertions, ok := assertRoute[routeKey]
 				if !ok {
 					// By default, all omitted routes check for just "authorize" called
 					routeAssertions = RouteCheck{}
 				}
+				delete(routeMissing, routeKey)
 
 				// Replace all url params with known values
-				route = strings.ReplaceAll(route, "{organization}", a.Admin.OrganizationID.String())
-				route = strings.ReplaceAll(route, "{user}", a.Admin.UserID.String())
-				route = strings.ReplaceAll(route, "{organizationname}", a.Organization.Name)
-				route = strings.ReplaceAll(route, "{workspace}", a.Workspace.ID.String())
-				route = strings.ReplaceAll(route, "{workspacebuild}", a.Workspace.LatestBuild.ID.String())
-				route = strings.ReplaceAll(route, "{workspacename}", a.Workspace.Name)
-				route = strings.ReplaceAll(route, "{workspacebuildname}", a.Workspace.LatestBuild.Name)
-				route = strings.ReplaceAll(route, "{workspaceagent}", a.WorkspaceResource.Agents[0].ID.String())
-				route = strings.ReplaceAll(route, "{buildnumber}", strconv.FormatInt(int64(a.Workspace.LatestBuild.BuildNumber), 10))
-				route = strings.ReplaceAll(route, "{template}", a.Template.ID.String())
-				route = strings.ReplaceAll(route, "{hash}", a.File.Hash)
-				route = strings.ReplaceAll(route, "{workspaceresource}", a.WorkspaceResource.ID.String())
-				route = strings.ReplaceAll(route, "{workspaceapp}", a.WorkspaceResource.Agents[0].Apps[0].Name)
-				route = strings.ReplaceAll(route, "{templateversion}", a.Version.ID.String())
-				route = strings.ReplaceAll(route, "{templateversiondryrun}", a.TemplateVersionDryRun.ID.String())
-				route = strings.ReplaceAll(route, "{templatename}", a.Template.Name)
-				// Only checking template scoped params here
-				route = strings.ReplaceAll(route, "{scope}", string(a.TemplateParam.Scope))
-				route = strings.ReplaceAll(route, "{id}", a.TemplateParam.ScopeID.String())
+				for k, v := range a.URLParams {
+					route = strings.ReplaceAll(route, k, v)
+				}
 
 				resp, err := a.Client.Request(ctx, method, route, nil)
 				require.NoError(t, err, "do req")
@@ -486,6 +495,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
 			return nil
 		})
 	require.NoError(a.t, err)
+	require.Len(a.t, routeMissing, 0, "didn't walk some asserted routes: %v", routeMissing)
 }
 
 type authCall struct {

coderd/database/databasefake/databasefake.go

@@ -2301,6 +2301,20 @@ func (q *fakeQuerier) GetLicenses(_ context.Context) ([]database.License, error)
 	return results, nil
 }
 
+func (q *fakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) {
+	q.mutex.Lock()
+	defer q.mutex.Unlock()
+
+	for index, l := range q.licenses {
+		if l.ID == id {
+			q.licenses[index] = q.licenses[len(q.licenses)-1]
+			q.licenses = q.licenses[:len(q.licenses)-1]
+			return id, nil
+		}
+	}
+	return 0, sql.ErrNoRows
+}
+
 func (q *fakeQuerier) GetUserLinkByLinkedID(_ context.Context, id string) (database.UserLink, error) {
 	q.mutex.RLock()
 	defer q.mutex.RUnlock()

coderd/database/querier.go

@@ -21,6 +21,7 @@ type querier interface {
 	AcquireProvisionerJob(ctx context.Context, arg AcquireProvisionerJobParams) (ProvisionerJob, error)
 	DeleteAPIKeyByID(ctx context.Context, id string) error
 	DeleteGitSSHKey(ctx context.Context, userID uuid.UUID) error
+	DeleteLicense(ctx context.Context, id int32) (int32, error)
 	DeleteParameterValueByID(ctx context.Context, id uuid.UUID) error
 	GetAPIKeyByID(ctx context.Context, id string) (APIKey, error)
 	GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Time) ([]APIKey, error)

coderd/database/queries.sql.go

@@ -475,6 +475,19 @@ func (q *sqlQuerier) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyPar
 	return err
 }
 
+const deleteLicense = `-- name: DeleteLicense :one
+DELETE
+FROM licenses
+WHERE id = $1
+RETURNING id
+`
+
+func (q *sqlQuerier) DeleteLicense(ctx context.Context, id int32) (int32, error) {
+	row := q.db.QueryRowContext(ctx, deleteLicense, id)
+	err := row.Scan(&id)
+	return id, err
+}
+
 const getLicenses = `-- name: GetLicenses :many
 SELECT id, uploaded_at, jwt, exp
 FROM licenses

coderd/database/queries/licenses.sql

@@ -8,8 +8,13 @@ INSERT INTO
 VALUES
 	($1, $2, $3) RETURNING *;
 
-
 -- name: GetLicenses :many
 SELECT *
 FROM licenses
 ORDER BY (id);
+
+-- name: DeleteLicense :one
+DELETE
+FROM licenses
+WHERE id = $1
+RETURNING id;

codersdk/licenses.go

@@ -3,6 +3,7 @@ package codersdk
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"time"
 )
@@ -50,3 +51,15 @@ func (c *Client) Licenses(ctx context.Context) ([]License, error) {
 	d.UseNumber()
 	return licenses, d.Decode(&licenses)
 }
+
+func (c *Client) DeleteLicense(ctx context.Context, id int32) error {
+	res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/licenses/%d", id), nil)
+	if err != nil {
+		return err
+	}
+	defer res.Body.Close()
+	if res.StatusCode != http.StatusOK {
+		return readBodyAsError(res)
+	}
+	return nil
+}

enterprise/coderd/auth_internal_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/ed25519"
 	"crypto/rand"
+	"fmt"
 	"net/http"
 	"testing"
 	"time"
@@ -55,10 +56,11 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 	}
 	lic, err := makeLicense(claims, privKey, keyID)
 	require.NoError(t, err)
-	_, err = a.Client.AddLicense(ctx, codersdk.AddLicenseRequest{
+	license, err := a.Client.AddLicense(ctx, codersdk.AddLicenseRequest{
 		License: lic,
 	})
 	require.NoError(t, err)
+	a.URLParams["licenses/{id}"] = fmt.Sprintf("licenses/%d", license.ID)
 
 	skipRoutes, assertRoute := coderdtest.AGPLRoutes(a)
 	assertRoute["POST:/api/v2/licenses"] = coderdtest.RouteCheck{
@@ -70,5 +72,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 		AssertAction: rbac.ActionRead,
 		AssertObject: rbac.ResourceLicense,
 	}
+	assertRoute["DELETE:/api/v2/licenses/{id}"] = coderdtest.RouteCheck{
+		AssertAction: rbac.ActionDelete,
+		AssertObject: rbac.ResourceLicense,
+	}
 	a.Test(ctx, assertRoute, skipRoutes)
 }

enterprise/coderd/licenses.go

@@ -9,6 +9,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"net/http"
+	"strconv"
 	"strings"
 	"time"
 
@@ -125,6 +126,7 @@ func newLicenseAPI(
 	a := &licenseAPI{router: r, logger: l, database: db, pubsub: ps, auth: auth}
 	r.Post("/", a.postLicense)
 	r.Get("/", a.licenses)
+	r.Delete("/{id}", a.delete)
 	return a
 }
 
@@ -265,3 +267,35 @@ func decodeClaims(l database.License) (jwt.MapClaims, error) {
 	err = d.Decode(&c)
 	return c, err
 }
+
+func (a *licenseAPI) delete(rw http.ResponseWriter, r *http.Request) {
+	if !a.auth.Authorize(r, rbac.ActionDelete, rbac.ResourceLicense) {
+		httpapi.Forbidden(rw)
+		return
+	}
+
+	idStr := chi.URLParam(r, "id")
+	id, err := strconv.ParseInt(idStr, 10, 32)
+	if err != nil {
+		httpapi.Write(rw, http.StatusNotFound, codersdk.Response{
+			Message: "License ID must be an integer",
+		})
+		return
+	}
+
+	_, err = a.database.DeleteLicense(r.Context(), int32(id))
+	if xerrors.Is(err, sql.ErrNoRows) {
+		httpapi.Write(rw, http.StatusNotFound, codersdk.Response{
+			Message: "Unknown license ID",
+		})
+		return
+	}
+	if err != nil {
+		httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Internal error deleting license",
+			Detail:  err.Error(),
+		})
+		return
+	}
+	rw.WriteHeader(http.StatusOK)
+}

enterprise/coderd/licenses_internal_test.go

@@ -5,6 +5,7 @@ import (
 	"crypto/ed25519"
 	"crypto/rand"
 	"encoding/json"
+	"net/http"
 	"testing"
 	"time"
 
@@ -212,6 +213,98 @@ func TestGetLicense(t *testing.T) {
 	})
 }
 
+// these tests patch the map of license keys, so cannot be run in parallel
+// nolint:paralleltest
+func TestDeleteLicense(t *testing.T) {
+	pubKey, privKey, err := ed25519.GenerateKey(rand.Reader)
+	require.NoError(t, err)
+	keyID := "testing"
+	oldKeys := keys
+	defer func() {
+		t.Log("restoring keys")
+		keys = oldKeys
+	}()
+	keys = map[string]ed25519.PublicKey{keyID: pubKey}
+
+	t.Run("DELETE_empty", func(t *testing.T) {
+		client := coderdtest.New(t, &coderdtest.Options{APIBuilder: NewEnterprise})
+		_ = coderdtest.CreateFirstUser(t, client)
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		err := client.DeleteLicense(ctx, 1)
+		errResp := &codersdk.Error{}
+		if xerrors.As(err, &errResp) {
+			assert.Equal(t, 404, errResp.StatusCode())
+		} else {
+			t.Error("expected to get error status 404")
+		}
+	})
+
+	t.Run("DELETE_bad_id", func(t *testing.T) {
+		client := coderdtest.New(t, &coderdtest.Options{APIBuilder: NewEnterprise})
+		_ = coderdtest.CreateFirstUser(t, client)
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		resp, err := client.Request(ctx, http.MethodDelete, "/api/v2/licenses/drivers", nil)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+		require.NoError(t, resp.Body.Close())
+	})
+
+	t.Run("DELETE", func(t *testing.T) {
+		client := coderdtest.New(t, &coderdtest.Options{APIBuilder: NewEnterprise})
+		_ = coderdtest.CreateFirstUser(t, client)
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		claims := &Claims{
+			RegisteredClaims: jwt.RegisteredClaims{
+				Issuer:    "test@coder.test",
+				IssuedAt:  jwt.NewNumericDate(time.Now()),
+				NotBefore: jwt.NewNumericDate(time.Now()),
+				ExpiresAt: jwt.NewNumericDate(time.Now().Add(2 * time.Hour)),
+			},
+			LicenseExpires: jwt.NewNumericDate(time.Now().Add(time.Hour)),
+			AccountType:    AccountTypeSalesforce,
+			AccountID:      "testing",
+			Version:        CurrentVersion,
+			Features: Features{
+				UserLimit: 0,
+				AuditLog:  1,
+			},
+		}
+		lic, err := makeLicense(claims, privKey, keyID)
+		require.NoError(t, err)
+		_, err = client.AddLicense(ctx, codersdk.AddLicenseRequest{
+			License: lic,
+		})
+		require.NoError(t, err)
+
+		// 2nd license
+		claims.AccountID = "testing2"
+		claims.Features.UserLimit = 200
+		lic2, err := makeLicense(claims, privKey, keyID)
+		require.NoError(t, err)
+		_, err = client.AddLicense(ctx, codersdk.AddLicenseRequest{
+			License: lic2,
+		})
+		require.NoError(t, err)
+
+		licenses, err := client.Licenses(ctx)
+		require.NoError(t, err)
+		assert.Len(t, licenses, 2)
+		for _, l := range licenses {
+			err = client.DeleteLicense(ctx, l.ID)
+			require.NoError(t, err)
+		}
+		licenses, err = client.Licenses(ctx)
+		require.NoError(t, err)
+		assert.Len(t, licenses, 0)
+	})
+}
+
 func makeLicense(c *Claims, privateKey ed25519.PrivateKey, keyID string) (string, error) {
 	tok := jwt.NewWithClaims(jwt.SigningMethodEdDSA, c)
 	tok.Header[HeaderKeyID] = keyID