Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: ensure proper rbac permissions on 'Acquire' file in the cache#18348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Emyrk wants to merge14 commits intomain
base:main
Choose a base branch
Loading
fromstevenmasley/file_cache_perms
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
14 commits
Select commitHold shift + click to select a range
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletionscoderd/authorize.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,7 +19,7 @@ import (
// objects that the user is authorized to perform the given action on.
// This is faster than calling Authorize() on each object.
func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action policy.Action, objects []O) ([]O, error) {
roles := httpmw.UserAuthorization(r)
roles := httpmw.UserAuthorization(r.Context())
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles, action, objects)
if err != nil {
// Log the error as Filter should not be erroring.
Expand DownExpand Up@@ -65,7 +65,7 @@ func (api *API) Authorize(r *http.Request, action policy.Action, object rbac.Obj
//return
//}
func (h *HTTPAuthorizer) Authorize(r *http.Request, action policy.Action, object rbac.Objecter) bool {
roles := httpmw.UserAuthorization(r)
roles := httpmw.UserAuthorization(r.Context())
err := h.Authorizer.Authorize(r.Context(), roles, action, object.RBACObject())
if err != nil {
// Log the errors for debugging
Expand DownExpand Up@@ -97,7 +97,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action policy.Action, object
// call 'Authorize()' on the returned objects.
// Note the authorization is only for the given action and object type.
func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action policy.Action, objectType string) (rbac.PreparedAuthorized, error) {
roles := httpmw.UserAuthorization(r)
roles := httpmw.UserAuthorization(r.Context())
prepared, err := h.Authorizer.Prepare(r.Context(), roles, action, objectType)
if err != nil {
return nil, xerrors.Errorf("prepare filter: %w", err)
Expand All@@ -120,7 +120,7 @@ func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action policy.Actio
// @Router /authcheck [post]
func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
auth := httpmw.UserAuthorization(r)
auth := httpmw.UserAuthorization(r.Context())

var params codersdk.AuthorizationRequest
if !httpapi.Read(ctx, rw, r, &params) {
Expand Down
2 changes: 1 addition & 1 deletioncoderd/coderd.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -572,7 +572,7 @@ func New(options *Options) *API {
TemplateScheduleStore: options.TemplateScheduleStore,
UserQuietHoursScheduleStore: options.UserQuietHoursScheduleStore,
AccessControlStore: options.AccessControlStore,
FileCache: files.NewFromStore(options.Database, options.PrometheusRegistry),
FileCache: files.NewFromStore(options.Database, options.PrometheusRegistry, options.Authorizer),
Experiments: experiments,
WebpushDispatcher: options.WebPushDispatcher,
healthCheckGroup: &singleflight.Group[string, *healthsdk.HealthcheckReport]{},
Expand Down
6 changes: 5 additions & 1 deletioncoderd/coderdtest/authorize.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -234,6 +234,10 @@ func (r *RecordingAuthorizer) AssertOutOfOrder(t *testing.T, actor rbac.Subject,
// AssertActor asserts in order. If the order of authz calls does not match,
// this will fail.
func (r *RecordingAuthorizer) AssertActor(t *testing.T, actor rbac.Subject, did ...ActionObjectPair) {
r.AssertActorID(t, actor.ID, did...)
}

func (r *RecordingAuthorizer) AssertActorID(t *testing.T, id string, did ...ActionObjectPair) {
r.Lock()
defer r.Unlock()
ptr := 0
Expand All@@ -242,7 +246,7 @@ func (r *RecordingAuthorizer) AssertActor(t *testing.T, actor rbac.Subject, did
// Finished all assertions
return
}
if call.Actor.ID ==actor.ID {
if call.Actor.ID ==id {
action, object := did[ptr].Action, did[ptr].Object
assert.Equalf(t, action, call.Action, "assert action %d", ptr)
assert.Equalf(t, object, call.Object, "assert object %d", ptr)
Expand Down
23 changes: 23 additions & 0 deletionscoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -432,6 +432,25 @@ var (
}),
Scope: rbac.ScopeAll,
}.WithCachedASTValue()

subjectFileReader = rbac.Subject{
Type: rbac.SubjectTypeFileReader,
FriendlyName: "Can Read All Files",
// Arbitrary uuid to have a unique ID for this subject.
ID: rbac.SubjectTypeFileReaderID,
Roles: rbac.Roles([]rbac.Role{
{
Identifier: rbac.RoleIdentifier{Name: "file-reader"},
DisplayName: "FileReader",
Site: rbac.Permissions(map[string][]policy.Action{
rbac.ResourceFile.Type: {policy.ActionRead},
}),
Org: map[string][]rbac.Permission{},
User: []rbac.Permission{},
},
}),
Scope: rbac.ScopeAll,
}.WithCachedASTValue()
)

// AsProvisionerd returns a context with an actor that has permissions required
Expand DownExpand Up@@ -498,6 +517,10 @@ func AsPrebuildsOrchestrator(ctx context.Context) context.Context {
return As(ctx, subjectPrebuildsOrchestrator)
}

func AsFileReader(ctx context.Context) context.Context {
return As(ctx, subjectFileReader)
}

var AsRemoveActor = rbac.Subject{
ID: "remove-actor",
}
Expand Down
57 changes: 39 additions & 18 deletionscoderd/files/cache.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,33 +13,41 @@ import (

archivefs "github.com/coder/coder/v2/archive/fs"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
"github.com/coder/coder/v2/coderd/util/lazy"
)

// NewFromStore returns a file cache that will fetch files from the provided
// database.
func NewFromStore(store database.Store, registerer prometheus.Registerer) *Cache {
fetch := func(ctx context.Context, fileID uuid.UUID) (cacheEntryValue, error) {
file, err := store.GetFileByID(ctx, fileID)
func NewFromStore(store database.Store, registerer prometheus.Registerer, authz rbac.Authorizer) *Cache {
fetch := func(ctx context.Context, fileID uuid.UUID) (CacheEntryValue, error) {
// Make sure the read does not fail due to authorization issues.
// Authz is checked on the Acquire call, so this is safe.
//nolint:gocritic
file, err := store.GetFileByID(dbauthz.AsFileReader(ctx), fileID)
if err != nil {
returncacheEntryValue{}, xerrors.Errorf("failed to read file from database: %w", err)
returnCacheEntryValue{}, xerrors.Errorf("failed to read file from database: %w", err)
}

content := bytes.NewBuffer(file.Data)
return cacheEntryValue{
FS: archivefs.FromTarReader(content),
size: int64(content.Len()),
return CacheEntryValue{
Object: file.RBACObject(),
FS: archivefs.FromTarReader(content),
Size: int64(content.Len()),
}, nil
}

return New(fetch, registerer)
return New(fetch, registerer, authz)
}

func New(fetch fetcher, registerer prometheus.Registerer) *Cache {
func New(fetch fetcher, registerer prometheus.Registerer, authz rbac.Authorizer) *Cache {
return (&Cache{
lock: sync.Mutex{},
data: make(map[uuid.UUID]*cacheEntry),
fetcher: fetch,
authz: authz,
}).registerMetrics(registerer)
}

Expand DownExpand Up@@ -101,6 +109,7 @@ type Cache struct {
lock sync.Mutex
data map[uuid.UUID]*cacheEntry
fetcher
authz rbac.Authorizer

// metrics
cacheMetrics
Expand All@@ -117,18 +126,19 @@ type cacheMetrics struct {
totalCacheSize prometheus.Counter
}

type cacheEntryValue struct {
type CacheEntryValue struct {
Object rbac.Object
fs.FS
Comment on lines +130 to 131
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
Objectrbac.Object
fs.FS
fs.FS
Objectrbac.Object

nit

size int64
Size int64
}

type cacheEntry struct {
// refCount must only be accessed while the Cache lock is held.
refCount int
value *lazy.ValueWithError[cacheEntryValue]
value *lazy.ValueWithError[CacheEntryValue]
}

type fetcher func(context.Context, uuid.UUID) (cacheEntryValue, error)
type fetcher func(context.Context, uuid.UUID) (CacheEntryValue, error)

// Acquire will load the fs.FS for the given file. It guarantees that parallel
// calls for the same fileID will only result in one fetch, and that parallel
Expand All@@ -146,22 +156,33 @@ func (c *Cache) Acquire(ctx context.Context, fileID uuid.UUID) (fs.FS, error) {
c.Release(fileID)
return nil, err
}

subject, ok := dbauthz.ActorFromContext(ctx)
if !ok {
return nil, dbauthz.ErrNoActor
}
// Always check the caller can actually read the file.
if err := c.authz.Authorize(ctx, subject, policy.ActionRead, it.Object); err != nil {
c.Release(fileID)
return nil, err
}

return it.FS, err
}

func (c *Cache) prepare(ctx context.Context, fileID uuid.UUID) *lazy.ValueWithError[cacheEntryValue] {
func (c *Cache) prepare(ctx context.Context, fileID uuid.UUID) *lazy.ValueWithError[CacheEntryValue] {
c.lock.Lock()
defer c.lock.Unlock()

entry, ok := c.data[fileID]
if !ok {
value := lazy.NewWithError(func() (cacheEntryValue, error) {
value := lazy.NewWithError(func() (CacheEntryValue, error) {
val, err := c.fetcher(ctx, fileID)

// Always add to the cache size the bytes of the file loaded.
if err == nil {
c.currentCacheSize.Add(float64(val.size))
c.totalCacheSize.Add(float64(val.size))
c.currentCacheSize.Add(float64(val.Size))
c.totalCacheSize.Add(float64(val.Size))
}

return val, err
Expand DownExpand Up@@ -206,7 +227,7 @@ func (c *Cache) Release(fileID uuid.UUID) {

ev, err := entry.value.Load()
if err == nil {
c.currentCacheSize.Add(-1 * float64(ev.size))
c.currentCacheSize.Add(-1 * float64(ev.Size))
}

delete(c.data, fileID)
Expand Down
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp