mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: add template setting to require active template version (#10277)
This commit is contained in:
@@ -0,0 +1,37 @@
|
||||
package dbauthz
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
)
|
||||
|
||||
// AccessControlStore fetches access control-related configuration
|
||||
// that is used when determining whether an actor is authorized
|
||||
// to interact with an RBAC object.
|
||||
type AccessControlStore interface {
|
||||
GetTemplateAccessControl(t database.Template) TemplateAccessControl
|
||||
SetTemplateAccessControl(ctx context.Context, store database.Store, id uuid.UUID, opts TemplateAccessControl) error
|
||||
}
|
||||
|
||||
type TemplateAccessControl struct {
|
||||
RequireActiveVersion bool
|
||||
}
|
||||
|
||||
// AGPLTemplateAccessControlStore always returns the defaults for access control
|
||||
// settings.
|
||||
type AGPLTemplateAccessControlStore struct{}
|
||||
|
||||
var _ AccessControlStore = AGPLTemplateAccessControlStore{}
|
||||
|
||||
func (AGPLTemplateAccessControlStore) GetTemplateAccessControl(database.Template) TemplateAccessControl {
|
||||
return TemplateAccessControl{
|
||||
RequireActiveVersion: false,
|
||||
}
|
||||
}
|
||||
|
||||
func (AGPLTemplateAccessControlStore) SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, TemplateAccessControl) error {
|
||||
return nil
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
@@ -101,9 +102,10 @@ type querier struct {
|
||||
db database.Store
|
||||
auth rbac.Authorizer
|
||||
log slog.Logger
|
||||
acs *atomic.Pointer[AccessControlStore]
|
||||
}
|
||||
|
||||
func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger) database.Store {
|
||||
func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger, acs *atomic.Pointer[AccessControlStore]) database.Store {
|
||||
// If the underlying db store is already a querier, return it.
|
||||
// Do not double wrap.
|
||||
if slices.Contains(db.Wrappers(), wrapname) {
|
||||
@@ -113,6 +115,7 @@ func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger) data
|
||||
db: db,
|
||||
auth: authorizer,
|
||||
log: logger,
|
||||
acs: acs,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -507,7 +510,7 @@ func (q *querier) Ping(ctx context.Context) (time.Duration, error) {
|
||||
func (q *querier) InTx(function func(querier database.Store) error, txOpts *sql.TxOptions) error {
|
||||
return q.db.InTx(func(tx database.Store) error {
|
||||
// Wrap the transaction store in a querier.
|
||||
wrapped := New(tx, q.auth, q.log)
|
||||
wrapped := New(tx, q.auth, q.log, q.acs)
|
||||
return function(wrapped)
|
||||
}, txOpts)
|
||||
}
|
||||
@@ -2200,7 +2203,7 @@ func (q *querier) InsertWorkspaceAppStats(ctx context.Context, arg database.Inse
|
||||
func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertWorkspaceBuildParams) error {
|
||||
w, err := q.db.GetWorkspaceByID(ctx, arg.WorkspaceID)
|
||||
if err != nil {
|
||||
return err
|
||||
return xerrors.Errorf("get workspace by id: %w", err)
|
||||
}
|
||||
|
||||
var action rbac.Action = rbac.ActionUpdate
|
||||
@@ -2209,7 +2212,28 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
|
||||
}
|
||||
|
||||
if err = q.authorizeContext(ctx, action, w.WorkspaceBuildRBAC(arg.Transition)); err != nil {
|
||||
return err
|
||||
return xerrors.Errorf("authorize context: %w", err)
|
||||
}
|
||||
|
||||
// If we're starting a workspace we need to check the template.
|
||||
if arg.Transition == database.WorkspaceTransitionStart {
|
||||
t, err := q.db.GetTemplateByID(ctx, w.TemplateID)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get template by id: %w", err)
|
||||
}
|
||||
|
||||
accessControl := (*q.acs.Load()).GetTemplateAccessControl(t)
|
||||
|
||||
// If the template requires the active version we need to check if
|
||||
// the user is a template admin. If they aren't and are attempting
|
||||
// to use a non-active version then we must fail the request.
|
||||
if accessControl.RequireActiveVersion {
|
||||
if arg.TemplateVersionID != t.ActiveVersionID {
|
||||
if err = q.authorizeContext(ctx, rbac.ActionUpdate, t); err != nil {
|
||||
return xerrors.Errorf("cannot use non-active version: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return q.db.InsertWorkspaceBuild(ctx, arg)
|
||||
@@ -2442,6 +2466,13 @@ func (q *querier) UpdateTemplateACLByID(ctx context.Context, arg database.Update
|
||||
return fetchAndExec(q.log, q.auth, rbac.ActionCreate, fetch, q.db.UpdateTemplateACLByID)(ctx, arg)
|
||||
}
|
||||
|
||||
func (q *querier) UpdateTemplateAccessControlByID(ctx context.Context, arg database.UpdateTemplateAccessControlByIDParams) error {
|
||||
fetch := func(ctx context.Context, arg database.UpdateTemplateAccessControlByIDParams) (database.Template, error) {
|
||||
return q.db.GetTemplateByID(ctx, arg.ID)
|
||||
}
|
||||
return update(q.log, q.auth, fetch, q.db.UpdateTemplateAccessControlByID)(ctx, arg)
|
||||
}
|
||||
|
||||
func (q *querier) UpdateTemplateActiveVersionByID(ctx context.Context, arg database.UpdateTemplateActiveVersionByIDParams) error {
|
||||
fetch := func(ctx context.Context, arg database.UpdateTemplateActiveVersionByIDParams) (database.Template, error) {
|
||||
return q.db.GetTemplateByID(ctx, arg.ID)
|
||||
|
||||
@@ -21,6 +21,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/database/dbtime"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/coderd/util/slice"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
func TestAsNoActor(t *testing.T) {
|
||||
@@ -61,7 +62,7 @@ func TestAsNoActor(t *testing.T) {
|
||||
func TestPing(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
q := dbauthz.New(dbfake.New(), &coderdtest.RecordingAuthorizer{}, slog.Make())
|
||||
q := dbauthz.New(dbfake.New(), &coderdtest.RecordingAuthorizer{}, slog.Make(), accessControlStorePointer())
|
||||
_, err := q.Ping(context.Background())
|
||||
require.NoError(t, err, "must not error")
|
||||
}
|
||||
@@ -73,7 +74,7 @@ func TestInTX(t *testing.T) {
|
||||
db := dbfake.New()
|
||||
q := dbauthz.New(db, &coderdtest.RecordingAuthorizer{
|
||||
Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: xerrors.New("custom error")},
|
||||
}, slog.Make())
|
||||
}, slog.Make(), accessControlStorePointer())
|
||||
actor := rbac.Subject{
|
||||
ID: uuid.NewString(),
|
||||
Roles: rbac.RoleNames{rbac.RoleOwner()},
|
||||
@@ -109,8 +110,8 @@ func TestNew(t *testing.T) {
|
||||
|
||||
// Double wrap should not cause an actual double wrap. So only 1 rbac call
|
||||
// should be made.
|
||||
az := dbauthz.New(db, rec, slog.Make())
|
||||
az = dbauthz.New(az, rec, slog.Make())
|
||||
az := dbauthz.New(db, rec, slog.Make(), accessControlStorePointer())
|
||||
az = dbauthz.New(az, rec, slog.Make(), accessControlStorePointer())
|
||||
|
||||
w, err := az.GetWorkspaceByID(ctx, exp.ID)
|
||||
require.NoError(t, err, "must not error")
|
||||
@@ -127,7 +128,7 @@ func TestDBAuthzRecursive(t *testing.T) {
|
||||
t.Parallel()
|
||||
q := dbauthz.New(dbfake.New(), &coderdtest.RecordingAuthorizer{
|
||||
Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: nil},
|
||||
}, slog.Make())
|
||||
}, slog.Make(), accessControlStorePointer())
|
||||
actor := rbac.Subject{
|
||||
ID: uuid.NewString(),
|
||||
Roles: rbac.RoleNames{rbac.RoleOwner()},
|
||||
@@ -1213,13 +1214,67 @@ func (s *MethodTestSuite) TestWorkspace() {
|
||||
}).Asserts(rbac.ResourceWorkspace.WithOwner(u.ID.String()).InOrg(o.ID), rbac.ActionCreate)
|
||||
}))
|
||||
s.Run("Start/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
|
||||
w := dbgen.Workspace(s.T(), db, database.Workspace{})
|
||||
t := dbgen.Template(s.T(), db, database.Template{})
|
||||
w := dbgen.Workspace(s.T(), db, database.Workspace{
|
||||
TemplateID: t.ID,
|
||||
})
|
||||
check.Args(database.InsertWorkspaceBuildParams{
|
||||
WorkspaceID: w.ID,
|
||||
Transition: database.WorkspaceTransitionStart,
|
||||
Reason: database.BuildReasonInitiator,
|
||||
}).Asserts(w.WorkspaceBuildRBAC(database.WorkspaceTransitionStart), rbac.ActionUpdate)
|
||||
}))
|
||||
s.Run("Start/RequireActiveVersion/VersionMismatch/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
|
||||
t := dbgen.Template(s.T(), db, database.Template{})
|
||||
ctx := testutil.Context(s.T(), testutil.WaitShort)
|
||||
err := db.UpdateTemplateAccessControlByID(ctx, database.UpdateTemplateAccessControlByIDParams{
|
||||
ID: t.ID,
|
||||
RequireActiveVersion: true,
|
||||
})
|
||||
require.NoError(s.T(), err)
|
||||
v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{
|
||||
TemplateID: uuid.NullUUID{UUID: t.ID},
|
||||
})
|
||||
w := dbgen.Workspace(s.T(), db, database.Workspace{
|
||||
TemplateID: t.ID,
|
||||
})
|
||||
check.Args(database.InsertWorkspaceBuildParams{
|
||||
WorkspaceID: w.ID,
|
||||
Transition: database.WorkspaceTransitionStart,
|
||||
Reason: database.BuildReasonInitiator,
|
||||
TemplateVersionID: v.ID,
|
||||
}).Asserts(
|
||||
w.WorkspaceBuildRBAC(database.WorkspaceTransitionStart), rbac.ActionUpdate,
|
||||
t, rbac.ActionUpdate,
|
||||
)
|
||||
}))
|
||||
s.Run("Start/RequireActiveVersion/VersionsMatch/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
|
||||
v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{})
|
||||
t := dbgen.Template(s.T(), db, database.Template{
|
||||
ActiveVersionID: v.ID,
|
||||
})
|
||||
|
||||
ctx := testutil.Context(s.T(), testutil.WaitShort)
|
||||
err := db.UpdateTemplateAccessControlByID(ctx, database.UpdateTemplateAccessControlByIDParams{
|
||||
ID: t.ID,
|
||||
RequireActiveVersion: true,
|
||||
})
|
||||
require.NoError(s.T(), err)
|
||||
|
||||
w := dbgen.Workspace(s.T(), db, database.Workspace{
|
||||
TemplateID: t.ID,
|
||||
})
|
||||
// Assert that we do not check for template update permissions
|
||||
// if versions match.
|
||||
check.Args(database.InsertWorkspaceBuildParams{
|
||||
WorkspaceID: w.ID,
|
||||
Transition: database.WorkspaceTransitionStart,
|
||||
Reason: database.BuildReasonInitiator,
|
||||
TemplateVersionID: v.ID,
|
||||
}).Asserts(
|
||||
w.WorkspaceBuildRBAC(database.WorkspaceTransitionStart), rbac.ActionUpdate,
|
||||
)
|
||||
}))
|
||||
s.Run("Delete/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
|
||||
w := dbgen.Workspace(s.T(), db, database.Workspace{})
|
||||
check.Args(database.InsertWorkspaceBuildParams{
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/golang/mock/gomock"
|
||||
@@ -59,7 +60,7 @@ func (s *MethodTestSuite) SetupSuite() {
|
||||
mockStore := dbmock.NewMockStore(ctrl)
|
||||
// We intentionally set no expectations apart from this.
|
||||
mockStore.EXPECT().Wrappers().Return([]string{}).AnyTimes()
|
||||
az := dbauthz.New(mockStore, nil, slog.Make())
|
||||
az := dbauthz.New(mockStore, nil, slog.Make(), accessControlStorePointer())
|
||||
// Take the underlying type of the interface.
|
||||
azt := reflect.TypeOf(az).Elem()
|
||||
s.methodAccounting = make(map[string]int)
|
||||
@@ -110,7 +111,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
|
||||
rec := &coderdtest.RecordingAuthorizer{
|
||||
Wrapped: fakeAuthorizer,
|
||||
}
|
||||
az := dbauthz.New(db, rec, slog.Make())
|
||||
az := dbauthz.New(db, rec, slog.Make(), accessControlStorePointer())
|
||||
actor := rbac.Subject{
|
||||
ID: uuid.NewString(),
|
||||
Roles: rbac.RoleNames{rbac.RoleOwner()},
|
||||
@@ -398,3 +399,22 @@ func (emptyPreparedAuthorized) Authorize(_ context.Context, _ rbac.Object) error
|
||||
func (emptyPreparedAuthorized) CompileToSQL(_ context.Context, _ regosql.ConvertConfig) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
|
||||
func accessControlStorePointer() *atomic.Pointer[dbauthz.AccessControlStore] {
|
||||
acs := &atomic.Pointer[dbauthz.AccessControlStore]{}
|
||||
var tacs dbauthz.AccessControlStore = fakeAccessControlStore{}
|
||||
acs.Store(&tacs)
|
||||
return acs
|
||||
}
|
||||
|
||||
type fakeAccessControlStore struct{}
|
||||
|
||||
func (fakeAccessControlStore) GetTemplateAccessControl(t database.Template) dbauthz.TemplateAccessControl {
|
||||
return dbauthz.TemplateAccessControl{
|
||||
RequireActiveVersion: t.RequireActiveVersion,
|
||||
}
|
||||
}
|
||||
|
||||
func (fakeAccessControlStore) SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, dbauthz.TemplateAccessControl) error {
|
||||
panic("not implemented")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user