diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index 8d0b5494568f8..42a67e92a8559 100644 --- a/coderd/audit/diff.go +++ b/coderd/audit/diff.go @@ -15,7 +15,8 @@ type Auditable interface { database.TemplateVersion | database.User | database.Workspace | - database.GitSSHKey + database.GitSSHKey | + database.Group } // Map is a map of changed fields in an audited resource. It maps field names to diff --git a/coderd/audit/request.go b/coderd/audit/request.go index c23ad2b1f7339..c658a09038569 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -45,6 +45,8 @@ func ResourceTarget[T Auditable](tgt T) string { return typed.Name case database.GitSSHKey: return typed.PublicKey + case database.Group: + return typed.Name default: panic(fmt.Sprintf("unknown resource %T", tgt)) } @@ -64,6 +66,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID { return typed.ID case database.GitSSHKey: return typed.UserID + case database.Group: + return typed.ID default: panic(fmt.Sprintf("unknown resource %T", tgt)) } @@ -83,6 +87,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType { return database.ResourceTypeWorkspace case database.GitSSHKey: return database.ResourceTypeGitSshKey + case database.Group: + return database.ResourceTypeGroup default: panic(fmt.Sprintf("unknown resource %T", tgt)) } diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 6f809ad117226..ad839dd08ed2a 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1457,34 +1457,6 @@ func (q *fakeQuerier) GetTemplates(_ context.Context) ([]database.Template, erro return templates, nil } -func (q *fakeQuerier) UpdateTemplateUserACLByID(_ context.Context, id uuid.UUID, acl database.TemplateACL) error { - q.mutex.RLock() - defer q.mutex.RUnlock() - - for i, t := range q.templates { - if t.ID == id { - t = t.SetUserACL(acl) - q.templates[i] = t - return nil - } - } - return sql.ErrNoRows -} - -func (q *fakeQuerier) UpdateTemplateGroupACLByID(_ context.Context, id uuid.UUID, acl database.TemplateACL) error { - q.mutex.RLock() - defer q.mutex.RUnlock() - - for i, t := range q.templates { - if t.ID == id { - t = t.SetGroupACL(acl) - q.templates[i] = t - return nil - } - } - return sql.ErrNoRows -} - func (q *fakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]database.TemplateUser, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -1501,10 +1473,8 @@ func (q *fakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]d return nil, sql.ErrNoRows } - acl := template.UserACL() - - users := make([]database.TemplateUser, 0, len(acl)) - for k, v := range acl { + users := make([]database.TemplateUser, 0, len(template.UserACL)) + for k, v := range template.UserACL { user, err := q.GetUserByID(context.Background(), uuid.MustParse(k)) if err != nil && xerrors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get user by ID: %w", err) @@ -1544,10 +1514,8 @@ func (q *fakeQuerier) GetTemplateGroupRoles(_ context.Context, id uuid.UUID) ([] return nil, sql.ErrNoRows } - acl := template.GroupACL() - - groups := make([]database.TemplateGroup, 0, len(acl)) - for k, v := range acl { + groups := make([]database.TemplateGroup, 0, len(template.GroupACL)) + for k, v := range template.GroupACL { group, err := q.GetGroupByID(context.Background(), uuid.MustParse(k)) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get group by ID: %w", err) @@ -2047,11 +2015,9 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl MaxTtl: arg.MaxTtl, MinAutostartInterval: arg.MinAutostartInterval, CreatedBy: arg.CreatedBy, + UserACL: arg.UserACL, + GroupACL: arg.GroupACL, } - template = template.SetUserACL(database.TemplateACL{}) - template = template.SetGroupACL(database.TemplateACL{ - arg.OrganizationID.String(): []rbac.Action{rbac.ActionRead}, - }) q.templates = append(q.templates, template) return template, nil } @@ -2470,6 +2436,23 @@ func (q *fakeQuerier) UpdateTemplateDeletedByID(_ context.Context, arg database. return sql.ErrNoRows } +func (q *fakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.UpdateTemplateACLByIDParams) (database.Template, error) { + q.mutex.Lock() + defer q.mutex.Unlock() + + for i, template := range q.templates { + if template.ID == arg.ID { + template.GroupACL = arg.GroupACL + template.UserACL = arg.UserACL + + q.templates[i] = template + return template, nil + } + } + + return database.Template{}, sql.ErrNoRows +} + func (q *fakeQuerier) UpdateTemplateVersionByID(_ context.Context, arg database.UpdateTemplateVersionByIDParams) error { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/drivers.go b/coderd/database/drivers.go index b0bf0f79612c8..a1084d1229662 100644 --- a/coderd/database/drivers.go +++ b/coderd/database/drivers.go @@ -24,3 +24,22 @@ func (a *Actions) Scan(src interface{}) error { func (a *Actions) Value() (driver.Value, error) { return json.Marshal(a) } + +// TemplateACL is a map of ids to permissions. +type TemplateACL map[string][]rbac.Action + +func (t *TemplateACL) Scan(src interface{}) error { + switch v := src.(type) { + case string: + return json.Unmarshal([]byte(v), &t) + case []byte, json.RawMessage: + //nolint + return json.Unmarshal(v.([]byte), &t) + } + + return xerrors.Errorf("unexpected type %T", src) +} + +func (t TemplateACL) Value() (driver.Value, error) { + return json.Marshal(t) +} diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index f68f8194f6dc2..f3710e91f037b 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -87,7 +87,8 @@ CREATE TYPE resource_type AS ENUM ( 'user', 'workspace', 'git_ssh_key', - 'api_key' + 'api_key', + 'group' ); CREATE TYPE user_status AS ENUM ( diff --git a/coderd/database/migrations/000063_resource_type_group.down.sql b/coderd/database/migrations/000063_resource_type_group.down.sql new file mode 100644 index 0000000000000..0d2e97aef130e --- /dev/null +++ b/coderd/database/migrations/000063_resource_type_group.down.sql @@ -0,0 +1,2 @@ +-- You cannot safely remove values from enums https://www.postgresql.org/docs/current/datatype-enum.html +-- You cannot create a new type and do a rename because objects depend on this type now. diff --git a/coderd/database/migrations/000063_resource_type_group.up.sql b/coderd/database/migrations/000063_resource_type_group.up.sql new file mode 100644 index 0000000000000..3234c61bb7ca1 --- /dev/null +++ b/coderd/database/migrations/000063_resource_type_group.up.sql @@ -0,0 +1,5 @@ +BEGIN; + +ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'group'; + +COMMIT; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index f12d8b7f2dceb..523d6e0afea25 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -1,65 +1,11 @@ package database import ( - "encoding/json" - "fmt" - "github.com/coder/coder/coderd/rbac" ) const AllUsersGroup = "Everyone" -// TemplateACL is a map of user_ids to permissions. -type TemplateACL map[string][]rbac.Action - -func (t Template) UserACL() TemplateACL { - var acl TemplateACL - if len(t.userACL) == 0 { - return acl - } - - err := json.Unmarshal(t.userACL, &acl) - if err != nil { - panic(fmt.Sprintf("failed to unmarshal template.userACL: %v", err.Error())) - } - - return acl -} - -func (t Template) GroupACL() TemplateACL { - var acl TemplateACL - if len(t.groupACL) == 0 { - return acl - } - - err := json.Unmarshal(t.groupACL, &acl) - if err != nil { - panic(fmt.Sprintf("failed to unmarshal template.userACL: %v", err.Error())) - } - - return acl -} - -func (t Template) SetGroupACL(acl TemplateACL) Template { - raw, err := json.Marshal(acl) - if err != nil { - panic(fmt.Sprintf("marshal user acl: %v", err)) - } - - t.groupACL = raw - return t -} - -func (t Template) SetUserACL(acl TemplateACL) Template { - raw, err := json.Marshal(acl) - if err != nil { - panic(fmt.Sprintf("marshal user acl: %v", err)) - } - - t.userACL = raw - return t -} - func (s APIKeyScope) ToRBAC() rbac.Scope { switch s { case APIKeyScopeAll: @@ -74,8 +20,8 @@ func (s APIKeyScope) ToRBAC() rbac.Scope { func (t Template) RBACObject() rbac.Object { obj := rbac.ResourceTemplate return obj.InOrg(t.OrganizationID). - WithACLUserList(t.UserACL()). - WithGroupACL(t.GroupACL()) + WithACLUserList(t.UserACL). + WithGroupACL(t.GroupACL) } func (TemplateVersion) RBACObject(template Template) rbac.Object { diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 3383b6af96e39..8ccbe5f14f6ca 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -2,7 +2,6 @@ package database import ( "context" - "encoding/json" "fmt" "strings" @@ -23,8 +22,6 @@ type customQuerier interface { } type templateQuerier interface { - UpdateTemplateUserACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error - UpdateTemplateGroupACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]TemplateGroup, error) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]TemplateUser, error) } @@ -34,28 +31,6 @@ type TemplateUser struct { Actions Actions `db:"actions"` } -func (q *sqlQuerier) UpdateTemplateUserACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error { - raw, err := json.Marshal(acl) - if err != nil { - return xerrors.Errorf("marshal user acl: %w", err) - } - - const query = ` -UPDATE - templates -SET - user_acl = $2 -WHERE - id = $1` - - _, err = q.db.ExecContext(ctx, query, id.String(), raw) - if err != nil { - return xerrors.Errorf("update user acl: %w", err) - } - - return nil -} - func (q *sqlQuerier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]TemplateUser, error) { const query = ` SELECT @@ -100,28 +75,6 @@ type TemplateGroup struct { Actions Actions `db:"actions"` } -func (q *sqlQuerier) UpdateTemplateGroupACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error { - raw, err := json.Marshal(acl) - if err != nil { - return xerrors.Errorf("marshal user acl: %w", err) - } - - const query = ` -UPDATE - templates -SET - group_acl = $2 -WHERE - id = $1` - - _, err = q.db.ExecContext(ctx, query, id.String(), raw) - if err != nil { - return xerrors.Errorf("update user acl: %w", err) - } - - return nil -} - func (q *sqlQuerier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]TemplateGroup, error) { const query = ` SELECT diff --git a/coderd/database/models.go b/coderd/database/models.go index 7e398552de93e..dd0bce6064514 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -301,6 +301,7 @@ const ( ResourceTypeWorkspace ResourceType = "workspace" ResourceTypeGitSshKey ResourceType = "git_ssh_key" ResourceTypeApiKey ResourceType = "api_key" + ResourceTypeGroup ResourceType = "group" ) func (e *ResourceType) Scan(src interface{}) error { @@ -573,8 +574,8 @@ type Template struct { MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` Icon string `db:"icon" json:"icon"` - userACL json.RawMessage `db:"user_acl" json:"user_acl"` - groupACL json.RawMessage `db:"group_acl" json:"group_acl"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` } type TemplateVersion struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 393ab81fdd347..16cb6f3151653 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -162,6 +162,7 @@ type sqlcQuerier interface { UpdateProvisionerJobWithCancelByID(ctx context.Context, arg UpdateProvisionerJobWithCancelByIDParams) error UpdateProvisionerJobWithCompleteByID(ctx context.Context, arg UpdateProvisionerJobWithCompleteByIDParams) error UpdateReplica(ctx context.Context, arg UpdateReplicaParams) (Replica, error) + UpdateTemplateACLByID(ctx context.Context, arg UpdateTemplateACLByIDParams) (Template, error) UpdateTemplateActiveVersionByID(ctx context.Context, arg UpdateTemplateActiveVersionByIDParams) error UpdateTemplateDeletedByID(ctx context.Context, arg UpdateTemplateDeletedByIDParams) error UpdateTemplateMetaByID(ctx context.Context, arg UpdateTemplateMetaByIDParams) (Template, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index cb4b43591a41e..16a1109636a53 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2899,8 +2899,8 @@ func (q *sqlQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (Templat &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ) return i, err } @@ -2941,8 +2941,8 @@ func (q *sqlQuerier) GetTemplateByOrganizationAndName(ctx context.Context, arg G &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ) return i, err } @@ -2975,8 +2975,8 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ); err != nil { return nil, err } @@ -3055,8 +3055,8 @@ func (q *sqlQuerier) GetTemplatesWithFilter(ctx context.Context, arg GetTemplate &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ); err != nil { return nil, err } @@ -3085,10 +3085,12 @@ INSERT INTO max_ttl, min_autostart_interval, created_by, - icon + icon, + user_acl, + group_acl ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl ` type InsertTemplateParams struct { @@ -3104,6 +3106,8 @@ type InsertTemplateParams struct { MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` Icon string `db:"icon" json:"icon"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` } func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParams) (Template, error) { @@ -3120,6 +3124,8 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam arg.MinAutostartInterval, arg.CreatedBy, arg.Icon, + arg.UserACL, + arg.GroupACL, ) var i Template err := row.Scan( @@ -3136,8 +3142,49 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, + ) + return i, err +} + +const updateTemplateACLByID = `-- name: UpdateTemplateACLByID :one +UPDATE + templates +SET + group_acl = $1, + user_acl = $2 +WHERE + id = $3 +RETURNING + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl +` + +type UpdateTemplateACLByIDParams struct { + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateTemplateACLByID(ctx context.Context, arg UpdateTemplateACLByIDParams) (Template, error) { + row := q.db.QueryRowContext(ctx, updateTemplateACLByID, arg.GroupACL, arg.UserACL, arg.ID) + var i Template + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OrganizationID, + &i.Deleted, + &i.Name, + &i.Provisioner, + &i.ActiveVersionID, + &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, + &i.CreatedBy, + &i.Icon, + &i.UserACL, + &i.GroupACL, ) return i, err } @@ -3235,8 +3282,8 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ) return i, err } diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 06570fffb0998..196355ad72404 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -68,10 +68,12 @@ INSERT INTO max_ttl, min_autostart_interval, created_by, - icon + icon, + user_acl, + group_acl ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING *; -- name: UpdateTemplateActiveVersionByID :exec UPDATE @@ -106,6 +108,17 @@ WHERE RETURNING *; +-- name: UpdateTemplateACLByID :one +UPDATE + templates +SET + group_acl = $1, + user_acl = $2 +WHERE + id = $3 +RETURNING + *; + -- name: GetTemplateAverageBuildTime :one WITH build_times AS ( SELECT diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 24df14057fcc9..3a6d6e55b6853 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -19,6 +19,12 @@ packages: overrides: - column: "users.rbac_roles" go_type: "github.com/lib/pq.StringArray" + - column: "templates.user_acl" + go_type: + type: "TemplateACL" + - column: "templates.group_acl" + go_type: + type: "TemplateACL" rename: api_key: APIKey @@ -39,5 +45,5 @@ rename: ip_addresses: IPAddresses ids: IDs jwt: JWT - user_acl: userACL - group_acl: groupACL + user_acl: UserACL + group_acl: GroupACL diff --git a/coderd/templates.go b/coderd/templates.go index 7b08303dea096..a1108dfc43082 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -257,6 +257,10 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque MaxTtl: int64(maxTTL), MinAutostartInterval: int64(minAutostartInterval), CreatedBy: apiKey.UserID, + UserACL: database.TemplateACL{}, + GroupACL: database.TemplateACL{ + organization.ID.String(): []rbac.Action{rbac.ActionRead}, + }, }) if err != nil { return xerrors.Errorf("insert template: %s", err) @@ -299,13 +303,6 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque } } - err = tx.UpdateTemplateGroupACLByID(ctx, dbTemplate.ID, database.TemplateACL{ - dbTemplate.OrganizationID.String(): []rbac.Action{rbac.ActionRead}, - }) - if err != nil { - return xerrors.Errorf("update template group acl: %w", err) - } - createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, tx, []database.Template{dbTemplate}) if err != nil { return xerrors.Errorf("get creator name: %w", err) @@ -683,6 +680,10 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO MaxTtl: int64(maxTTLDefault), MinAutostartInterval: int64(minAutostartIntervalDefault), CreatedBy: opts.userID, + UserACL: database.TemplateACL{}, + GroupACL: database.TemplateACL{ + opts.orgID.String(): []rbac.Action{rbac.ActionRead}, + }, }) if err != nil { return xerrors.Errorf("insert template: %w", err) @@ -718,13 +719,6 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO } } - err = tx.UpdateTemplateGroupACLByID(ctx, template.ID, database.TemplateACL{ - opts.orgID.String(): []rbac.Action{rbac.ActionRead}, - }) - if err != nil { - return xerrors.Errorf("update template group acl: %w", err) - } - return nil }) diff --git a/codersdk/audit.go b/codersdk/audit.go index 068d8d63c9b3d..1452af0c0f6b9 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -21,6 +21,7 @@ const ( ResourceTypeWorkspace ResourceType = "workspace" ResourceTypeGitSSHKey ResourceType = "git_ssh_key" ResourceTypeAPIKey ResourceType = "api_key" + ResourceTypeGroup ResourceType = "group" ) func (r ResourceType) FriendlyString() string { @@ -39,6 +40,8 @@ func (r ResourceType) FriendlyString() string { return "git ssh key" case ResourceTypeAPIKey: return "api key" + case ResourceTypeGroup: + return "group" default: return "unknown" } diff --git a/enterprise/audit/diff.go b/enterprise/audit/diff.go index dac3ab437bdcf..05d46499b525a 100644 --- a/enterprise/audit/diff.go +++ b/enterprise/audit/diff.go @@ -8,6 +8,7 @@ import ( "github.com/google/uuid" "github.com/coder/coder/coderd/audit" + "github.com/coder/coder/coderd/database" ) func structName(t reflect.Type) string { @@ -137,7 +138,8 @@ func convertDiffType(left, right any) (newLeft, newRight any, changed bool) { } return leftInt64Ptr, rightInt64Ptr, true - + case database.TemplateACL: + return fmt.Sprintf("%+v", left), fmt.Sprintf("%+v", right), true default: return left, right, false } diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 0a0ebeca7304f..54a21a986200a 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -62,6 +62,8 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "min_autostart_interval": ActionTrack, "created_by": ActionTrack, "is_private": ActionTrack, + "group_acl": ActionTrack, + "user_acl": ActionTrack, }, &database.TemplateVersion{}: { "id": ActionTrack, @@ -101,6 +103,12 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "ttl": ActionTrack, "last_used_at": ActionIgnore, }, + &database.Group{}: { + "id": ActionTrack, + "name": ActionTrack, + "organization_id": ActionTrack, + "avatar_url": ActionTrack, + }, }) // auditMap converts a map of struct pointers to a map of struct names as diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index a65b8e8faa6e0..9459cc6906a84 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -14,6 +14,8 @@ import ( "github.com/coder/coder/cli/deployment" "github.com/coder/coder/cryptorand" + "github.com/coder/coder/enterprise/audit" + "github.com/coder/coder/enterprise/audit/backends" "github.com/coder/coder/enterprise/coderd" "github.com/coder/coder/tailnet" @@ -48,6 +50,13 @@ func server() *cobra.Command { } options.DERPServer.SetMeshKey(meshKey) + if dflags.AuditLogging.Value { + options.Auditor = audit.NewAuditor(audit.DefaultFilter, + backends.NewPostgres(options.Database, true), + backends.NewSlog(options.Logger), + ) + } + o := &coderd.Options{ AuditLogging: dflags.AuditLogging.Value, BrowserOnly: dflags.BrowserOnly.Value, @@ -59,6 +68,7 @@ func server() *cobra.Command { Options: options, } + api, err := coderd.New(ctx, o) if err != nil { return nil, nil, err diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index a9c8c3b590228..b9d095883824e 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -22,8 +22,6 @@ import ( "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/workspacequota" "github.com/coder/coder/codersdk" - "github.com/coder/coder/enterprise/audit" - "github.com/coder/coder/enterprise/audit/backends" "github.com/coder/coder/enterprise/coderd/license" "github.com/coder/coder/enterprise/derpmesh" "github.com/coder/coder/enterprise/replicasync" @@ -241,11 +239,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { if changed, enabled := featureChanged(codersdk.FeatureAuditLog); changed { auditor := agplaudit.NewNop() if enabled { - auditor = audit.NewAuditor( - audit.DefaultFilter, - backends.NewPostgres(api.Database, true), - backends.NewSlog(api.Logger), - ) + auditor = api.AGPL.Options.Auditor } api.AGPL.Auditor.Store(&auditor) } diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 7b51845ff3986..a6c3c6d86973b 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -158,6 +158,9 @@ func TestAuditLogging(t *testing.T) { t.Parallel() client, _, api := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ AuditLogging: true, + Options: &coderdtest.Options{ + Auditor: audit.NewAuditor(audit.DefaultFilter), + }, }) coderdtest.CreateFirstUser(t, client) coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 7c1cb907062d9..6537602eb3b7d 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -9,6 +9,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/coderd" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -18,9 +19,17 @@ import ( func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - org = httpmw.OrganizationParam(r) + ctx = r.Context() + org = httpmw.OrganizationParam(r) + auditor = api.AGPL.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + }) ) + defer commitAudit() if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceGroup) { http.NotFound(rw, r) @@ -55,15 +64,25 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) httpapi.InternalServerError(rw, err) return } + aReq.New = group httpapi.Write(ctx, rw, http.StatusCreated, convertGroup(group, nil)) } func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - group = httpmw.GroupParam(r) + ctx = r.Context() + group = httpmw.GroupParam(r) + auditor = api.AGPL.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) ) + defer commitAudit() + aReq.Old = group if !api.Authorize(r, rbac.ActionUpdate, group) { http.NotFound(rw, r) @@ -195,14 +214,25 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { return } + aReq.New = group + httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, members)) } func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - group = httpmw.GroupParam(r) + ctx = r.Context() + group = httpmw.GroupParam(r) + auditor = api.AGPL.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionDelete, + }) ) + defer commitAudit() + aReq.Old = group if !api.Authorize(r, rbac.ActionDelete, group) { httpapi.ResourceNotFound(rw) diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 8019b956d4b04..0cbec41be2a9f 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/utils/pointer" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" @@ -39,6 +40,37 @@ func TestCreateGroup(t *testing.T) { require.NotEqual(t, uuid.Nil.String(), group.ID.String()) }) + t.Run("Audit", func(t *testing.T) { + t.Parallel() + + auditor := audit.NewMock() + client := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Auditor: auditor, + }, + }) + user := coderdtest.CreateFirstUser(t, client) + + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + TemplateRBAC: true, + AuditLog: true, + }) + + ctx, _ := testutil.Context(t) + + numLogs := len(auditor.AuditLogs) + group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + numLogs++ + require.Len(t, auditor.AuditLogs, numLogs) + require.Equal(t, database.AuditActionCreate, auditor.AuditLogs[numLogs-1].Action) + require.Equal(t, group.ID, auditor.AuditLogs[numLogs-1].ResourceID) + }) + t.Run("Conflict", func(t *testing.T) { t.Parallel() @@ -197,6 +229,43 @@ func TestPatchGroup(t *testing.T) { require.Contains(t, group.Members, user4) }) + t.Run("Audit", func(t *testing.T) { + t.Parallel() + + auditor := audit.NewMock() + client := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Auditor: auditor, + }, + }) + + user := coderdtest.CreateFirstUser(t, client) + + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + TemplateRBAC: true, + AuditLog: true, + }) + + ctx, _ := testutil.Context(t) + + group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + + numLogs := len(auditor.AuditLogs) + group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{ + Name: "bye", + }) + require.NoError(t, err) + numLogs++ + + require.Len(t, auditor.AuditLogs, numLogs) + require.Equal(t, database.AuditActionWrite, auditor.AuditLogs[numLogs-1].Action) + require.Equal(t, group.ID, auditor.AuditLogs[numLogs-1].ResourceID) + }) t.Run("NameConflict", func(t *testing.T) { t.Parallel() @@ -547,6 +616,40 @@ func TestDeleteGroup(t *testing.T) { require.Equal(t, http.StatusNotFound, cerr.StatusCode()) }) + t.Run("Audit", func(t *testing.T) { + t.Parallel() + + auditor := audit.NewMock() + client := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Auditor: auditor, + }, + }) + user := coderdtest.CreateFirstUser(t, client) + + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + TemplateRBAC: true, + AuditLog: true, + }) + ctx, _ := testutil.Context(t) + + group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + + numLogs := len(auditor.AuditLogs) + err = client.DeleteGroup(ctx, group.ID) + require.NoError(t, err) + numLogs++ + + require.Len(t, auditor.AuditLogs, numLogs) + require.Equal(t, database.AuditActionDelete, auditor.AuditLogs[numLogs-1].Action) + require.Equal(t, group.ID, auditor.AuditLogs[numLogs-1].ResourceID) + }) + t.Run("allUsers", func(t *testing.T) { t.Parallel() diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 245f8d1e1dbd6..9fbcb403735e8 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -10,6 +10,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/coderd" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -18,8 +19,11 @@ import ( ) func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - template := httpmw.TemplateParam(r) + var ( + ctx = r.Context() + template = httpmw.TemplateParam(r) + ) + if !api.Authorize(r, rbac.ActionRead, template) { httpapi.ResourceNotFound(rw) return @@ -90,9 +94,18 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) { var ( - ctx = r.Context() - template = httpmw.TemplateParam(r) + ctx = r.Context() + template = httpmw.TemplateParam(r) + auditor = api.AGPL.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) ) + defer commitAudit() + aReq.Old = template // Only users who are able to create templates (aka template admins) // are able to control permissions. @@ -119,40 +132,43 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) { } err := api.Database.InTx(func(tx database.Store) error { + var err error + template, err = tx.GetTemplateByID(ctx, template.ID) + if err != nil { + return xerrors.Errorf("get template by ID: %w", err) + } + if len(req.UserPerms) > 0 { - userACL := template.UserACL() for id, role := range req.UserPerms { // A user with an empty string implies // deletion. if role == "" { - delete(userACL, id) + delete(template.UserACL, id) continue } - userACL[id] = convertSDKTemplateRole(role) - } - - err := tx.UpdateTemplateUserACLByID(r.Context(), template.ID, userACL) - if err != nil { - return xerrors.Errorf("update template user ACL: %w", err) + template.UserACL[id] = convertSDKTemplateRole(role) } } if len(req.GroupPerms) > 0 { - groupACL := template.GroupACL() for id, role := range req.GroupPerms { // An id with an empty string implies // deletion. if role == "" { - delete(groupACL, id) + delete(template.GroupACL, id) continue } - groupACL[id] = convertSDKTemplateRole(role) + template.GroupACL[id] = convertSDKTemplateRole(role) } + } - err := tx.UpdateTemplateGroupACLByID(ctx, template.ID, groupACL) - if err != nil { - return xerrors.Errorf("update template user ACL: %w", err) - } + template, err = tx.UpdateTemplateACLByID(ctx, database.UpdateTemplateACLByIDParams{ + ID: template.ID, + UserACL: template.UserACL, + GroupACL: template.GroupACL, + }) + if err != nil { + return xerrors.Errorf("update template ACL by ID: %w", err) } return nil }) @@ -161,6 +177,8 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) { return } + aReq.New = template + httpapi.Write(ctx, rw, http.StatusOK, codersdk.Response{ Message: "Successfully updated template ACL list.", }) diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 87aa5a4ca83d8..ee8003c5532a5 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -8,7 +8,9 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" "github.com/coder/coder/enterprise/coderd/coderdenttest" "github.com/coder/coder/provisioner/echo" @@ -355,6 +357,46 @@ func TestUpdateTemplateACL(t *testing.T) { require.Contains(t, acl.Users, templateUser3) }) + t.Run("Audit", func(t *testing.T) { + t.Parallel() + + auditor := audit.NewMock() + client := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Auditor: auditor, + }, + }) + + user := coderdtest.CreateFirstUser(t, client) + + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + TemplateRBAC: true, + AuditLog: true, + }) + + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + ctx, _ := testutil.Context(t) + + numLogs := len(auditor.AuditLogs) + + req := codersdk.UpdateTemplateACL{ + GroupPerms: map[string]codersdk.TemplateRole{ + user.OrganizationID.String(): codersdk.TemplateRoleDeleted, + }, + } + err := client.UpdateTemplateACL(ctx, template.ID, req) + require.NoError(t, err) + numLogs++ + + require.Len(t, auditor.AuditLogs, numLogs) + require.Equal(t, database.AuditActionWrite, auditor.AuditLogs[numLogs-1].Action) + require.Equal(t, template.ID, auditor.AuditLogs[numLogs-1].ResourceID) + }) + t.Run("DeleteUser", func(t *testing.T) { t.Parallel() diff --git a/go.mod b/go.mod index 8fec641bc9ca1..758f1d6a4fff9 100644 --- a/go.mod +++ b/go.mod @@ -194,7 +194,7 @@ require ( github.com/fxamacker/cbor/v2 v2.4.0 // indirect github.com/ghodss/yaml v1.0.0 // indirect github.com/gin-gonic/gin v1.7.0 // indirect - github.com/go-logr/logr v1.2.3 // indirect + github.com/go-logr/logr v1.2.3 github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/go-playground/locales v0.14.0 // indirect diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 0a709f8927a2a..5b3c44bd0b979 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -940,6 +940,7 @@ export type ProvisionerType = "echo" | "terraform" export type ResourceType = | "api_key" | "git_ssh_key" + | "group" | "organization" | "template" | "template_version"