Skip to content

Commit 08cce81

Browse files
Emyrkjohnstcn
Emyrk
and
johnstcn
authored
feat: Implement allow_list for scopes for resource specific permissions (coder#5769)
* feat: Implement allow_list for scopes for resource specific permissions Feature that adds an allow_list for scopes to specify particular resources. This enables workspace agent tokens to use the same RBAC system as users. - Add ID to compileSQL matchers * Plumb through WithID on rbac objects * Rename Scope -> ScopeName * Update input.json with scope allow_list Co-authored-by: Cian Johnston <cian@coder.com>
1 parent f0df068 commit 08cce81

25 files changed

+444
-152
lines changed

‎coderd/apikey.go

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,9 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
143143
// @Router /users/{user}/keys/{keyid} [get]
144144
func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) {
145145
var (
146-
ctx = r.Context()
147-
user = httpmw.UserParam(r)
146+
ctx = r.Context()
148147
)
149148

150-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
151-
httpapi.ResourceNotFound(rw)
152-
return
153-
}
154-
155149
keyID := chi.URLParam(r, "keyid")
156150
key, err := api.Database.GetAPIKeyByID(ctx, keyID)
157151
if errors.Is(err, sql.ErrNoRows) {
@@ -166,6 +160,11 @@ func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) {
166160
return
167161
}
168162

163+
if !api.Authorize(r, rbac.ActionRead, key) {
164+
httpapi.ResourceNotFound(rw)
165+
return
166+
}
167+
169168
httpapi.Write(ctx, rw, http.StatusOK, convertAPIKey(key))
170169
}
171170

@@ -179,25 +178,28 @@ func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) {
179178
// @Router /users/{user}/keys/tokens [get]
180179
func (api *API) tokens(rw http.ResponseWriter, r *http.Request) {
181180
var (
182-
ctx = r.Context()
183-
user = httpmw.UserParam(r)
181+
ctx = r.Context()
184182
)
185183

186-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
187-
httpapi.ResourceNotFound(rw)
184+
keys, err := api.Database.GetAPIKeysByLoginType(ctx, database.LoginTypeToken)
185+
if err != nil {
186+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
187+
Message: "Internal error fetching API keys.",
188+
Detail: err.Error(),
189+
})
188190
return
189191
}
190192

191-
keys, err := api.Database.GetAPIKeysByLoginType(ctx, database.LoginTypeToken)
193+
keys, err = AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, keys)
192194
if err != nil {
193195
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
194-
Message: "Internal error fetching API keys.",
196+
Message: "Internal error fetching keys.",
195197
Detail: err.Error(),
196198
})
197199
return
198200
}
199201

200-
apiKeys := []codersdk.APIKey{}
202+
var apiKeys []codersdk.APIKey
201203
for _, key := range keys {
202204
apiKeys = append(apiKeys, convertAPIKey(key))
203205
}
@@ -215,16 +217,16 @@ func (api *API) tokens(rw http.ResponseWriter, r *http.Request) {
215217
// @Router /users/{user}/keys/{keyid} [delete]
216218
func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
217219
var (
218-
ctx = r.Context()
219-
user = httpmw.UserParam(r)
220+
ctx = r.Context()
221+
user = httpmw.UserParam(r)
222+
keyID = chi.URLParam(r, "keyid")
220223
)
221224

222-
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
225+
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceAPIKey.WithIDString(keyID).WithOwner(user.ID.String())) {
223226
httpapi.ResourceNotFound(rw)
224227
return
225228
}
226229

227-
keyID := chi.URLParam(r, "keyid")
228230
err := api.Database.DeleteAPIKeyByID(ctx, keyID)
229231
if errors.Is(err, sql.ErrNoRows) {
230232
httpapi.ResourceNotFound(rw)

‎coderd/coderdtest/authorize.go

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strconv"
99
"strings"
1010
"testing"
11+
"time"
1112

1213
"github.com/coder/coder/coderd/database/databasefake"
1314

@@ -32,9 +33,10 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
3233
_, isMemoryDB := a.api.Database.(databasefake.FakeDatabase)
3334

3435
// Some quick reused objects
35-
workspaceRBACObj := rbac.ResourceWorkspace.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
36-
workspaceExecObj := rbac.ResourceWorkspaceExecution.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
37-
applicationConnectObj := rbac.ResourceWorkspaceApplicationConnect.InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
36+
workspaceRBACObj := rbac.ResourceWorkspace.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
37+
workspaceExecObj := rbac.ResourceWorkspaceExecution.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
38+
applicationConnectObj := rbac.ResourceWorkspaceApplicationConnect.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
39+
templateObj := rbac.ResourceTemplate.WithID(a.Template.ID).InOrg(a.Template.OrganizationID)
3840

3941
// skipRoutes allows skipping routes from being checked.
4042
skipRoutes := map[string]string{
@@ -75,7 +77,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
7577
"POST:/api/v2/workspaceagents/me/report-stats": {NoAuthorize: true},
7678

7779
// These endpoints have more assertions. This is good, add more endpoints to assert if you can!
78-
"GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(a.Admin.OrganizationID)},
80+
"GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.WithID(a.Admin.OrganizationID).InOrg(a.Admin.OrganizationID)},
7981
"GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization},
8082
"GET:/api/v2/users/{user}/workspace/{workspacename}": {
8183
AssertObject: rbac.ResourceWorkspace,
@@ -85,6 +87,15 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
8587
AssertObject: rbac.ResourceWorkspace,
8688
AssertAction: rbac.ActionRead,
8789
},
90+
"GET:/api/v2/users/{user}/keys/tokens": {
91+
AssertObject: rbac.ResourceAPIKey,
92+
AssertAction: rbac.ActionRead,
93+
StatusCode: http.StatusOK,
94+
},
95+
"GET:/api/v2/users/{user}/keys/{keyid}": {
96+
AssertObject: rbac.ResourceAPIKey,
97+
AssertAction: rbac.ActionRead,
98+
},
8899
"GET:/api/v2/workspacebuilds/{workspacebuild}": {
89100
AssertAction: rbac.ActionRead,
90101
AssertObject: workspaceRBACObj,
@@ -139,11 +150,11 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
139150
},
140151
"DELETE:/api/v2/templates/{template}": {
141152
AssertAction: rbac.ActionDelete,
142-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
153+
AssertObject: templateObj,
143154
},
144155
"GET:/api/v2/templates/{template}": {
145156
AssertAction: rbac.ActionRead,
146-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
157+
AssertObject: templateObj,
147158
},
148159
"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
149160
"GET:/api/v2/files/{fileID}": {
@@ -152,64 +163,64 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
152163
},
153164
"GET:/api/v2/templates/{template}/versions": {
154165
AssertAction: rbac.ActionRead,
155-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
166+
AssertObject: templateObj,
156167
},
157168
"PATCH:/api/v2/templates/{template}/versions": {
158169
AssertAction: rbac.ActionUpdate,
159-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
170+
AssertObject: templateObj,
160171
},
161172
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {
162173
AssertAction: rbac.ActionRead,
163-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
174+
AssertObject: templateObj,
164175
},
165176
"GET:/api/v2/templateversions/{templateversion}": {
166177
AssertAction: rbac.ActionRead,
167-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
178+
AssertObject: templateObj,
168179
},
169180
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {
170181
AssertAction: rbac.ActionUpdate,
171-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
182+
AssertObject: templateObj,
172183
},
173184
"GET:/api/v2/templateversions/{templateversion}/logs": {
174185
AssertAction: rbac.ActionRead,
175-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
186+
AssertObject: templateObj,
176187
},
177188
"GET:/api/v2/templateversions/{templateversion}/parameters": {
178189
AssertAction: rbac.ActionRead,
179-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
190+
AssertObject: templateObj,
180191
},
181192
"GET:/api/v2/templateversions/{templateversion}/rich-parameters": {
182193
AssertAction: rbac.ActionRead,
183-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
194+
AssertObject: templateObj,
184195
},
185196
"GET:/api/v2/templateversions/{templateversion}/resources": {
186197
AssertAction: rbac.ActionRead,
187-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
198+
AssertObject: templateObj,
188199
},
189200
"GET:/api/v2/templateversions/{templateversion}/schema": {
190201
AssertAction: rbac.ActionRead,
191-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
202+
AssertObject: templateObj,
192203
},
193204
"POST:/api/v2/templateversions/{templateversion}/dry-run": {
194205
// The first check is to read the template
195206
AssertAction: rbac.ActionRead,
196-
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
207+
AssertObject: templateObj,
197208
},
198209
"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}": {
199210
AssertAction: rbac.ActionRead,
200-
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
211+
AssertObject: templateObj,
201212
},
202213
"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/resources": {
203214
AssertAction: rbac.ActionRead,
204-
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
215+
AssertObject: templateObj,
205216
},
206217
"GET:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/logs": {
207218
AssertAction: rbac.ActionRead,
208-
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
219+
AssertObject: templateObj,
209220
},
210221
"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{jobID}/cancel": {
211222
AssertAction: rbac.ActionRead,
212-
AssertObject: rbac.ResourceTemplate.InOrg(a.Version.OrganizationID),
223+
AssertObject: templateObj,
213224
},
214225
"POST:/api/v2/parameters/{scope}/{id}": {
215226
AssertAction: rbac.ActionUpdate,
@@ -225,7 +236,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
225236
},
226237
"GET:/api/v2/organizations/{organization}/templates/{templatename}": {
227238
AssertAction: rbac.ActionRead,
228-
AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID),
239+
AssertObject: templateObj,
229240
},
230241
"POST:/api/v2/organizations/{organization}/members/{user}/workspaces": {
231242
AssertAction: rbac.ActionCreate,
@@ -317,6 +328,15 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a
317328
if !ok {
318329
t.Fail()
319330
}
331+
_, err := client.CreateToken(ctx, admin.UserID.String(), codersdk.CreateTokenRequest{
332+
Lifetime: time.Hour,
333+
Scope: codersdk.APIKeyScopeAll,
334+
})
335+
require.NoError(t, err, "create token")
336+
337+
apiKeys, err := client.GetTokens(ctx, admin.UserID.String())
338+
require.NoError(t, err, "get tokens")
339+
apiKey := apiKeys[0]
320340

321341
organization, err := client.Organization(ctx, admin.OrganizationID)
322342
require.NoError(t, err, "fetch org")
@@ -383,6 +403,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a
383403
"{jobID}": templateVersionDryRun.ID.String(),
384404
"{templatename}": template.Name,
385405
"{workspace_and_agent}": workspace.Name + "." + workspace.LatestBuild.Resources[0].Agents[0].Name,
406+
"{keyid}": apiKey.ID,
386407
// Only checking template scoped params here
387408
"parameters/{scope}/{id}": fmt.Sprintf("parameters/%s/%s",
388409
string(templateParam.Scope), templateParam.ScopeID.String()),
@@ -507,7 +528,7 @@ type authCall struct {
507528
SubjectID string
508529
Roles []string
509530
Groups []string
510-
Scope rbac.Scope
531+
Scope rbac.ScopeName
511532
Action rbac.Action
512533
Object rbac.Object
513534
}
@@ -521,11 +542,11 @@ var _ rbac.Authorizer = (*RecordingAuthorizer)(nil)
521542

522543
// ByRoleNameSQL does not record the call. This matches the postgres behavior
523544
// of not calling Authorize()
524-
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.Scope, _ []string, _ rbac.Action, _ rbac.Object) error {
545+
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error {
525546
return r.AlwaysReturn
526547
}
527548

528-
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.Scope, groups []string, action rbac.Action, object rbac.Object) error {
549+
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error {
529550
r.Called = &authCall{
530551
SubjectID: subjectID,
531552
Roles: roleNames,
@@ -537,7 +558,7 @@ func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, ro
537558
return r.AlwaysReturn
538559
}
539560

540-
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.Scope, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
561+
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
541562
return &fakePreparedAuthorizer{
542563
Original: r,
543564
SubjectID: subjectID,
@@ -557,7 +578,7 @@ type fakePreparedAuthorizer struct {
557578
Original *RecordingAuthorizer
558579
SubjectID string
559580
Roles []string
560-
Scope rbac.Scope
581+
Scope rbac.ScopeName
561582
Action rbac.Action
562583
Groups []string
563584
HardCodedSQLString string

0 commit comments

Comments
 (0)