Skip to content

Commit ca38114

Browse files
spikecurtisspikecurtis
authored
DELETE license API endpoint (coder#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>
1 parent 14a9576 commit ca38114

File tree

9 files changed

+229
-40
lines changed

9 files changed

+229
-40
lines changed

‎coderd/coderdtest/authtest.go

Lines changed: 48 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package coderdtest
22

33
import (
44
"context"
5+
"fmt"
56
"io"
67
"net/http"
78
"strconv"
@@ -43,6 +44,7 @@ type AuthTester struct {
4344
File codersdk.UploadResponse
4445
TemplateVersionDryRun codersdk.ProvisionerJob
4546
TemplateParam codersdk.Parameter
47+
URLParams map[string]string
4648
}
4749

4850
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
8688
Id: "something",
8789
Auth: &proto.Agent_Token{},
8890
Apps: []*proto.App{{
89-
Name: "app",
91+
Name: "testapp",
9092
Url: "http://localhost:3000",
9193
}},
9294
}},
@@ -116,6 +118,28 @@ func NewAuthTester(ctx context.Context, t *testing.T, options *Options) *AuthTes
116118
})
117119
require.NoError(t, err, "create template param")
118120

121+
urlParameters := map[string]string{
122+
"{organization}": admin.OrganizationID.String(),
123+
"{user}": admin.UserID.String(),
124+
"{organizationname}": organization.Name,
125+
"{workspace}": workspace.ID.String(),
126+
"{workspacebuild}": workspace.LatestBuild.ID.String(),
127+
"{workspacename}": workspace.Name,
128+
"{workspacebuildname}": workspace.LatestBuild.Name,
129+
"{workspaceagent}": workspaceResources[0].Agents[0].ID.String(),
130+
"{buildnumber}": strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10),
131+
"{template}": template.ID.String(),
132+
"{hash}": file.Hash,
133+
"{workspaceresource}": workspaceResources[0].ID.String(),
134+
"{workspaceapp}": workspaceResources[0].Agents[0].Apps[0].Name,
135+
"{templateversion}": version.ID.String(),
136+
"{jobID}": templateVersionDryRun.ID.String(),
137+
"{templatename}": template.Name,
138+
// Only checking template scoped params here
139+
"parameters/{scope}/{id}": fmt.Sprintf("parameters/%s/%s",
140+
string(templateParam.Scope), templateParam.ScopeID.String()),
141+
}
142+
119143
return &AuthTester{
120144
t: t,
121145
api: api,
@@ -130,6 +154,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, options *Options) *AuthTes
130154
File: file,
131155
TemplateVersionDryRun: templateVersionDryRun,
132156
TemplateParam: templateParam,
157+
URLParams: urlParameters,
133158
}
134159
}
135160

@@ -153,13 +178,13 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
153178
"POST:/api/v2/csp/reports": {NoAuthorize: true},
154179
"GET:/api/v2/entitlements": {NoAuthorize: true},
155180

156-
"GET:/%40{user}/{workspacename}/apps/{application}/*": {
157-
AssertAction: rbac.ActionRead,
158-
AssertObject: workspaceRBACObj,
181+
"GET:/%40{user}/{workspacename}/apps/{workspaceapp}/*": {
182+
AssertAction: rbac.ActionCreate,
183+
AssertObject: workspaceExecObj,
159184
},
160-
"GET:/@{user}/{workspacename}/apps/{application}/*": {
161-
AssertAction: rbac.ActionRead,
162-
AssertObject: workspaceRBACObj,
185+
"GET:/@{user}/{workspacename}/apps/{workspaceapp}/*": {
186+
AssertAction: rbac.ActionCreate,
187+
AssertObject: workspaceExecObj,
163188
},
164189

165190
// Has it's own auth
@@ -188,7 +213,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
188213
AssertObject: rbac.ResourceWorkspace,
189214
AssertAction: rbac.ActionRead,
190215
},
191-
"GET:/api/v2/users/me/workspace/{workspacename}/builds/{buildnumber}": {
216+
"GET:/api/v2/users/{user}/workspace/{workspacename}/builds/{buildnumber}": {
192217
AssertObject: rbac.ResourceWorkspace,
193218
AssertAction: rbac.ActionRead,
194219
},
@@ -216,7 +241,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
216241
AssertAction: rbac.ActionUpdate,
217242
AssertObject: workspaceRBACObj,
218243
},
219-
"PUT:/api/v2/workspaces/{workspace}/autostop": {
244+
"PUT:/api/v2/workspaces/{workspace}/ttl": {
220245
AssertAction: rbac.ActionUpdate,
221246
AssertObject: workspaceRBACObj,
222247
},
@@ -275,7 +300,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
275300
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
276301
},
277302
"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
278-
"GET:/api/v2/files/{fileHash}": {
303+
"GET:/api/v2/files/{hash}": {
279304
AssertAction: rbac.ActionRead,
280305
AssertObject: rbac.ResourceFile.WithOwner(a.Admin.UserID.String()),
281306
},
@@ -320,19 +345,19 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
320345
AssertAction: rbac.ActionRead,
321346
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
322347
},
323-
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}": {
348+
"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}": {
324349
AssertAction: rbac.ActionRead,
325350
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
326351
},
327-
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/resources": {
352+
"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/resources": {
328353
AssertAction: rbac.ActionRead,
329354
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
330355
},
331-
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/logs": {
356+
"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/logs": {
332357
AssertAction: rbac.ActionRead,
333358
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
334359
},
335-
"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/cancel": {
360+
"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/cancel": {
336361
AssertAction: rbac.ActionRead,
337362
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
338363
},
@@ -366,10 +391,6 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
366391
AssertAction: rbac.ActionRead,
367392
AssertObject: workspaceRBACObj,
368393
},
369-
"POST:/api/v2/users/{user}/organizations": {
370-
AssertAction: rbac.ActionCreate,
371-
AssertObject: rbac.ResourceOrganization,
372-
},
373394
"GET:/api/v2/users": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceUser},
374395

375396
// These endpoints need payloads to get to the auth part. Payloads will be required
@@ -385,13 +406,15 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
385406
// Always fail auth from this point forward
386407
a.authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
387408

409+
routeMissing := make(map[string]bool)
388410
for k, v := range assertRoute {
389411
noTrailSlash := strings.TrimRight(k, "/")
390412
if _, ok := assertRoute[noTrailSlash]; ok && noTrailSlash != k {
391413
a.t.Errorf("route %q & %q is declared twice", noTrailSlash, k)
392414
a.t.FailNow()
393415
}
394416
assertRoute[noTrailSlash] = v
417+
routeMissing[noTrailSlash] = true
395418
}
396419

397420
for k, v := range skipRoutes {
@@ -422,32 +445,18 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
422445
}
423446
a.t.Run(name, func(t *testing.T) {
424447
a.authorizer.reset()
425-
routeAssertions, ok := assertRoute[strings.TrimRight(name, "/")]
448+
routeKey := strings.TrimRight(name, "/")
449+
routeAssertions, ok := assertRoute[routeKey]
426450
if !ok {
427451
// By default, all omitted routes check for just "authorize" called
428452
routeAssertions = RouteCheck{}
429453
}
454+
delete(routeMissing, routeKey)
430455

431456
// Replace all url params with known values
432-
route = strings.ReplaceAll(route, "{organization}", a.Admin.OrganizationID.String())
433-
route = strings.ReplaceAll(route, "{user}", a.Admin.UserID.String())
434-
route = strings.ReplaceAll(route, "{organizationname}", a.Organization.Name)
435-
route = strings.ReplaceAll(route, "{workspace}", a.Workspace.ID.String())
436-
route = strings.ReplaceAll(route, "{workspacebuild}", a.Workspace.LatestBuild.ID.String())
437-
route = strings.ReplaceAll(route, "{workspacename}", a.Workspace.Name)
438-
route = strings.ReplaceAll(route, "{workspacebuildname}", a.Workspace.LatestBuild.Name)
439-
route = strings.ReplaceAll(route, "{workspaceagent}", a.WorkspaceResource.Agents[0].ID.String())
440-
route = strings.ReplaceAll(route, "{buildnumber}", strconv.FormatInt(int64(a.Workspace.LatestBuild.BuildNumber), 10))
441-
route = strings.ReplaceAll(route, "{template}", a.Template.ID.String())
442-
route = strings.ReplaceAll(route, "{hash}", a.File.Hash)
443-
route = strings.ReplaceAll(route, "{workspaceresource}", a.WorkspaceResource.ID.String())
444-
route = strings.ReplaceAll(route, "{workspaceapp}", a.WorkspaceResource.Agents[0].Apps[0].Name)
445-
route = strings.ReplaceAll(route, "{templateversion}", a.Version.ID.String())
446-
route = strings.ReplaceAll(route, "{templateversiondryrun}", a.TemplateVersionDryRun.ID.String())
447-
route = strings.ReplaceAll(route, "{templatename}", a.Template.Name)
448-
// Only checking template scoped params here
449-
route = strings.ReplaceAll(route, "{scope}", string(a.TemplateParam.Scope))
450-
route = strings.ReplaceAll(route, "{id}", a.TemplateParam.ScopeID.String())
457+
for k, v := range a.URLParams {
458+
route = strings.ReplaceAll(route, k, v)
459+
}
451460

452461
resp, err := a.Client.Request(ctx, method, route, nil)
453462
require.NoError(t, err, "do req")
@@ -486,6 +495,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
486495
return nil
487496
})
488497
require.NoError(a.t, err)
498+
require.Len(a.t, routeMissing, 0, "didn't walk some asserted routes: %v", routeMissing)
489499
}
490500

491501
type authCall struct {

‎coderd/database/databasefake/databasefake.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2301,6 +2301,20 @@ func (q *fakeQuerier) GetLicenses(_ context.Context) ([]database.License, error)
23012301
return results, nil
23022302
}
23032303

2304+
func (q *fakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) {
2305+
q.mutex.Lock()
2306+
defer q.mutex.Unlock()
2307+
2308+
for index, l := range q.licenses {
2309+
if l.ID == id {
2310+
q.licenses[index] = q.licenses[len(q.licenses)-1]
2311+
q.licenses = q.licenses[:len(q.licenses)-1]
2312+
return id, nil
2313+
}
2314+
}
2315+
return 0, sql.ErrNoRows
2316+
}
2317+
23042318
func (q *fakeQuerier) GetUserLinkByLinkedID(_ context.Context, id string) (database.UserLink, error) {
23052319
q.mutex.RLock()
23062320
defer q.mutex.RUnlock()

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/licenses.sql

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,13 @@ INSERT INTO
88
VALUES
99
($1, $2, $3) RETURNING *;
1010

11-
1211
-- name: GetLicenses :many
1312
SELECT *
1413
FROM licenses
1514
ORDER BY (id);
15+
16+
-- name: DeleteLicense :one
17+
DELETE
18+
FROM licenses
19+
WHERE id = $1
20+
RETURNING id;

‎codersdk/licenses.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package codersdk
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"net/http"
78
"time"
89
)
@@ -50,3 +51,15 @@ func (c *Client) Licenses(ctx context.Context) ([]License, error) {
5051
d.UseNumber()
5152
return licenses, d.Decode(&licenses)
5253
}
54+
55+
func (c *Client) DeleteLicense(ctx context.Context, id int32) error {
56+
res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/licenses/%d", id), nil)
57+
if err != nil {
58+
return err
59+
}
60+
defer res.Body.Close()
61+
if res.StatusCode != http.StatusOK {
62+
return readBodyAsError(res)
63+
}
64+
return nil
65+
}

‎enterprise/coderd/auth_internal_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"crypto/ed25519"
66
"crypto/rand"
7+
"fmt"
78
"net/http"
89
"testing"
910
"time"
@@ -55,10 +56,11 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
5556
}
5657
lic, err := makeLicense(claims, privKey, keyID)
5758
require.NoError(t, err)
58-
_, err = a.Client.AddLicense(ctx, codersdk.AddLicenseRequest{
59+
license, err := a.Client.AddLicense(ctx, codersdk.AddLicenseRequest{
5960
License: lic,
6061
})
6162
require.NoError(t, err)
63+
a.URLParams["licenses/{id}"] = fmt.Sprintf("licenses/%d", license.ID)
6264

6365
skipRoutes, assertRoute := coderdtest.AGPLRoutes(a)
6466
assertRoute["POST:/api/v2/licenses"] = coderdtest.RouteCheck{
@@ -70,5 +72,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
7072
AssertAction: rbac.ActionRead,
7173
AssertObject: rbac.ResourceLicense,
7274
}
75+
assertRoute["DELETE:/api/v2/licenses/{id}"] = coderdtest.RouteCheck{
76+
AssertAction: rbac.ActionDelete,
77+
AssertObject: rbac.ResourceLicense,
78+
}
7379
a.Test(ctx, assertRoute, skipRoutes)
7480
}

‎enterprise/coderd/licenses.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"encoding/base64"
1010
"encoding/json"
1111
"net/http"
12+
"strconv"
1213
"strings"
1314
"time"
1415

@@ -125,6 +126,7 @@ func newLicenseAPI(
125126
a := &licenseAPI{router: r, logger: l, database: db, pubsub: ps, auth: auth}
126127
r.Post("/", a.postLicense)
127128
r.Get("/", a.licenses)
129+
r.Delete("/{id}", a.delete)
128130
return a
129131
}
130132

@@ -265,3 +267,35 @@ func decodeClaims(l database.License) (jwt.MapClaims, error) {
265267
err = d.Decode(&c)
266268
return c, err
267269
}
270+
271+
func (a *licenseAPI) delete(rw http.ResponseWriter, r *http.Request) {
272+
if !a.auth.Authorize(r, rbac.ActionDelete, rbac.ResourceLicense) {
273+
httpapi.Forbidden(rw)
274+
return
275+
}
276+
277+
idStr := chi.URLParam(r, "id")
278+
id, err := strconv.ParseInt(idStr, 10, 32)
279+
if err != nil {
280+
httpapi.Write(rw, http.StatusNotFound, codersdk.Response{
281+
Message: "License ID must be an integer",
282+
})
283+
return
284+
}
285+
286+
_, err = a.database.DeleteLicense(r.Context(), int32(id))
287+
if xerrors.Is(err, sql.ErrNoRows) {
288+
httpapi.Write(rw, http.StatusNotFound, codersdk.Response{
289+
Message: "Unknown license ID",
290+
})
291+
return
292+
}
293+
if err != nil {
294+
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
295+
Message: "Internal error deleting license",
296+
Detail: err.Error(),
297+
})
298+
return
299+
}
300+
rw.WriteHeader(http.StatusOK)
301+
}

0 commit comments

Comments
 (0)