mirror of https://github.com/coder/coder.git
feat: Add RBAC to /files endpoints (#1664)
* feat: Add RBAC to /files endpoints
This commit is contained in:
parent
f763472609
commit
5f8d0e5dad
|
@ -134,6 +134,7 @@ func newRouter(options *Options, a *api) chi.Router {
|
|||
r.Route("/files", func(r chi.Router) {
|
||||
r.Use(
|
||||
apiKeyMiddleware,
|
||||
authRolesMiddleware,
|
||||
// This number is arbitrary, but reading/writing
|
||||
// file content is expensive so it should be small.
|
||||
httpmw.RateLimitPerMinute(12),
|
||||
|
|
|
@ -16,6 +16,7 @@ import (
|
|||
"github.com/coder/coder/buildinfo"
|
||||
"github.com/coder/coder/coderd/coderdtest"
|
||||
"github.com/coder/coder/coderd/rbac"
|
||||
"github.com/coder/coder/codersdk"
|
||||
)
|
||||
|
||||
func TestMain(m *testing.M) {
|
||||
|
@ -34,6 +35,7 @@ func TestBuildInfo(t *testing.T) {
|
|||
// TestAuthorizeAllEndpoints will check `authorize` is called on every endpoint registered.
|
||||
func TestAuthorizeAllEndpoints(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
|
||||
authorizer := &fakeAuthorizer{}
|
||||
srv, client, _ := coderdtest.NewWithServer(t, &coderdtest.Options{
|
||||
|
@ -50,6 +52,8 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
|
|||
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
|
||||
workspace := coderdtest.CreateWorkspace(t, client, admin.OrganizationID, template.ID)
|
||||
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
|
||||
file, err := client.Upload(ctx, codersdk.ContentTypeTar, make([]byte, 1024))
|
||||
require.NoError(t, err, "upload file")
|
||||
|
||||
// Always fail auth from this point forward
|
||||
authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
|
||||
|
@ -121,8 +125,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
|
|||
|
||||
"POST:/api/v2/users/{user}/organizations": {NoAuthorize: true},
|
||||
|
||||
"POST:/api/v2/files": {NoAuthorize: true},
|
||||
"GET:/api/v2/files/{hash}": {NoAuthorize: true},
|
||||
"GET:/api/v2/workspaces/{workspace}/watch": {NoAuthorize: true},
|
||||
|
||||
// These endpoints have more assertions. This is good, add more endpoints to assert if you can!
|
||||
|
@ -184,6 +186,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
|
|||
AssertObject: workspaceRBACObj,
|
||||
},
|
||||
|
||||
"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
|
||||
"GET:/api/v2/files/{fileHash}": {AssertAction: rbac.ActionRead,
|
||||
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash)},
|
||||
|
||||
// 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},
|
||||
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
|
||||
|
@ -220,6 +226,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
|
|||
route = strings.ReplaceAll(route, "{workspacebuild}", workspace.LatestBuild.ID.String())
|
||||
route = strings.ReplaceAll(route, "{workspacename}", workspace.Name)
|
||||
route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name)
|
||||
route = strings.ReplaceAll(route, "{hash}", file.Hash)
|
||||
|
||||
resp, err := client.Request(context.Background(), method, route, nil)
|
||||
require.NoError(t, err, "do req")
|
||||
|
|
|
@ -14,11 +14,18 @@ import (
|
|||
"github.com/coder/coder/coderd/database"
|
||||
"github.com/coder/coder/coderd/httpapi"
|
||||
"github.com/coder/coder/coderd/httpmw"
|
||||
"github.com/coder/coder/coderd/rbac"
|
||||
"github.com/coder/coder/codersdk"
|
||||
)
|
||||
|
||||
func (api *api) postFile(rw http.ResponseWriter, r *http.Request) {
|
||||
apiKey := httpmw.APIKey(r)
|
||||
// This requires the site wide action to create files.
|
||||
// Once created, a user can read their own files uploaded
|
||||
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceFile) {
|
||||
return
|
||||
}
|
||||
|
||||
contentType := r.Header.Get("Content-Type")
|
||||
|
||||
switch contentType {
|
||||
|
@ -77,9 +84,7 @@ func (api *api) fileByHash(rw http.ResponseWriter, r *http.Request) {
|
|||
}
|
||||
file, err := api.Database.GetFileByHash(r.Context(), hash)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
httpapi.Write(rw, http.StatusNotFound, httpapi.Response{
|
||||
Message: "no file exists with that hash",
|
||||
})
|
||||
httpapi.Forbidden(rw)
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
|
@ -88,6 +93,12 @@ func (api *api) fileByHash(rw http.ResponseWriter, r *http.Request) {
|
|||
})
|
||||
return
|
||||
}
|
||||
|
||||
if !api.Authorize(rw, r, rbac.ActionRead,
|
||||
rbac.ResourceFile.WithOwner(file.CreatedBy.String()).WithID(file.Hash)) {
|
||||
return
|
||||
}
|
||||
|
||||
rw.Header().Set("Content-Type", file.Mimetype)
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
_, _ = rw.Write(file.Data)
|
||||
|
|
|
@ -50,7 +50,7 @@ func TestDownload(t *testing.T) {
|
|||
_, _, err := client.Download(context.Background(), "something")
|
||||
var apiErr *codersdk.Error
|
||||
require.ErrorAs(t, err, &apiErr)
|
||||
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
|
||||
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
|
||||
})
|
||||
|
||||
t.Run("Insert", func(t *testing.T) {
|
||||
|
|
Loading…
Reference in New Issue