fix: correct perms for forbidden error in TemplateScheduleStore.Load (#11286)

* chore: TemplateScheduleStore.Load() throwing forbidden error
* fix: workspace agent scope to include template
This commit is contained in:
Steven Masley 2023-12-20 11:38:49 -06:00 committed by GitHub
parent 20dff2aa5d
commit fe867d02e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 49 additions and 16 deletions

View File

@ -61,7 +61,7 @@ func (a *StatsAPI) UpdateStats(ctx context.Context, req *agentproto.UpdateStatsR
templateSchedule, err := (*(a.TemplateScheduleStore.Load())).Get(ctx, a.Database, workspace.TemplateID) templateSchedule, err := (*(a.TemplateScheduleStore.Load())).Get(ctx, a.Database, workspace.TemplateID)
// If the template schedule fails to load, just default to bumping without the next trasition and log it. // If the template schedule fails to load, just default to bumping without the next trasition and log it.
if err != nil { if err != nil {
a.Log.Warn(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min", a.Log.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
slog.F("workspace_id", workspace.ID), slog.F("workspace_id", workspace.ID),
slog.F("template_id", workspace.TemplateID), slog.F("template_id", workspace.TemplateID),
slog.Error(err), slog.Error(err),

View File

@ -3796,6 +3796,7 @@ func (q *FakeQuerier) GetWorkspaceAgentAndOwnerByAuthToken(_ context.Context, au
} }
var row database.GetWorkspaceAgentAndOwnerByAuthTokenRow var row database.GetWorkspaceAgentAndOwnerByAuthTokenRow
row.WorkspaceID = ws.ID row.WorkspaceID = ws.ID
row.TemplateID = ws.TemplateID
usr, err := q.getUserByIDNoLock(ws.OwnerID) usr, err := q.getUserByIDNoLock(ws.OwnerID)
if err != nil { if err != nil {
return database.GetWorkspaceAgentAndOwnerByAuthTokenRow{}, sql.ErrNoRows return database.GetWorkspaceAgentAndOwnerByAuthTokenRow{}, sql.ErrNoRows
@ -3805,6 +3806,7 @@ func (q *FakeQuerier) GetWorkspaceAgentAndOwnerByAuthToken(_ context.Context, au
// We also need to get org roles for the user // We also need to get org roles for the user
row.OwnerName = usr.Username row.OwnerName = usr.Username
row.WorkspaceAgent = agt row.WorkspaceAgent = agt
row.TemplateVersionID = build.TemplateVersionID
for _, mem := range q.organizationMembers { for _, mem := range q.organizationMembers {
if mem.UserID == usr.ID { if mem.UserID == usr.ID {
row.OwnerRoles = append(row.OwnerRoles, fmt.Sprintf("organization-member:%s", mem.OrganizationID.String())) row.OwnerRoles = append(row.OwnerRoles, fmt.Sprintf("organization-member:%s", mem.OrganizationID.String()))

View File

@ -7781,6 +7781,8 @@ SELECT
users.id AS owner_id, users.id AS owner_id,
users.username AS owner_name, users.username AS owner_name,
users.status AS owner_status, users.status AS owner_status,
workspaces.template_id AS template_id,
workspace_builds.template_version_id AS template_version_id,
array_cat( array_cat(
array_append(users.rbac_roles, 'member'), array_append(users.rbac_roles, 'member'),
array_append(ARRAY[]::text[], 'organization-member:' || organization_members.organization_id::text) array_append(ARRAY[]::text[], 'organization-member:' || organization_members.organization_id::text)
@ -7823,20 +7825,23 @@ GROUP BY
workspaces.id, workspaces.id,
users.id, users.id,
organization_members.organization_id, organization_members.organization_id,
workspace_builds.build_number workspace_builds.build_number,
workspace_builds.template_version_id
ORDER BY ORDER BY
workspace_builds.build_number DESC workspace_builds.build_number DESC
LIMIT 1 LIMIT 1
` `
type GetWorkspaceAgentAndOwnerByAuthTokenRow struct { type GetWorkspaceAgentAndOwnerByAuthTokenRow struct {
WorkspaceAgent WorkspaceAgent `db:"workspace_agent" json:"workspace_agent"` WorkspaceAgent WorkspaceAgent `db:"workspace_agent" json:"workspace_agent"`
WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"`
OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` OwnerID uuid.UUID `db:"owner_id" json:"owner_id"`
OwnerName string `db:"owner_name" json:"owner_name"` OwnerName string `db:"owner_name" json:"owner_name"`
OwnerStatus UserStatus `db:"owner_status" json:"owner_status"` OwnerStatus UserStatus `db:"owner_status" json:"owner_status"`
OwnerRoles []string `db:"owner_roles" json:"owner_roles"` TemplateID uuid.UUID `db:"template_id" json:"template_id"`
OwnerGroups []string `db:"owner_groups" json:"owner_groups"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"`
OwnerRoles []string `db:"owner_roles" json:"owner_roles"`
OwnerGroups []string `db:"owner_groups" json:"owner_groups"`
} }
func (q *sqlQuerier) GetWorkspaceAgentAndOwnerByAuthToken(ctx context.Context, authToken uuid.UUID) (GetWorkspaceAgentAndOwnerByAuthTokenRow, error) { func (q *sqlQuerier) GetWorkspaceAgentAndOwnerByAuthToken(ctx context.Context, authToken uuid.UUID) (GetWorkspaceAgentAndOwnerByAuthTokenRow, error) {
@ -7877,6 +7882,8 @@ func (q *sqlQuerier) GetWorkspaceAgentAndOwnerByAuthToken(ctx context.Context, a
&i.OwnerID, &i.OwnerID,
&i.OwnerName, &i.OwnerName,
&i.OwnerStatus, &i.OwnerStatus,
&i.TemplateID,
&i.TemplateVersionID,
pq.Array(&i.OwnerRoles), pq.Array(&i.OwnerRoles),
pq.Array(&i.OwnerGroups), pq.Array(&i.OwnerGroups),
) )

View File

@ -219,6 +219,8 @@ SELECT
users.id AS owner_id, users.id AS owner_id,
users.username AS owner_name, users.username AS owner_name,
users.status AS owner_status, users.status AS owner_status,
workspaces.template_id AS template_id,
workspace_builds.template_version_id AS template_version_id,
array_cat( array_cat(
array_append(users.rbac_roles, 'member'), array_append(users.rbac_roles, 'member'),
array_append(ARRAY[]::text[], 'organization-member:' || organization_members.organization_id::text) array_append(ARRAY[]::text[], 'organization-member:' || organization_members.organization_id::text)
@ -261,7 +263,8 @@ GROUP BY
workspaces.id, workspaces.id,
users.id, users.id,
organization_members.organization_id, organization_members.organization_id,
workspace_builds.build_number workspace_builds.build_number,
workspace_builds.template_version_id
ORDER BY ORDER BY
workspace_builds.build_number DESC workspace_builds.build_number DESC
LIMIT 1; LIMIT 1;

View File

@ -97,7 +97,12 @@ func ExtractWorkspaceAgent(opts ExtractWorkspaceAgentConfig) func(http.Handler)
ID: row.OwnerID.String(), ID: row.OwnerID.String(),
Roles: rbac.RoleNames(row.OwnerRoles), Roles: rbac.RoleNames(row.OwnerRoles),
Groups: row.OwnerGroups, Groups: row.OwnerGroups,
Scope: rbac.WorkspaceAgentScope(row.WorkspaceID, row.OwnerID), Scope: rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{
WorkspaceID: row.WorkspaceID,
OwnerID: row.OwnerID,
TemplateID: row.TemplateID,
VersionID: row.TemplateVersionID,
}),
}.WithCachedASTValue() }.WithCachedASTValue()
ctx = context.WithValue(ctx, workspaceAgentContextKey{}, row.WorkspaceAgent) ctx = context.WithValue(ctx, workspaceAgentContextKey{}, row.WorkspaceAgent)

View File

@ -394,12 +394,14 @@ func TestAgentStats(t *testing.T) {
require.NoError(t, err, "create stats batcher failed") require.NoError(t, err, "create stats batcher failed")
t.Cleanup(closeBatcher) t.Cleanup(closeBatcher)
tLogger := slogtest.Make(t, nil)
// Build sample workspaces with test agents and fake agent client // Build sample workspaces with test agents and fake agent client
client, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{ client, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{
Database: db, Database: db,
IncludeProvisionerDaemon: true, IncludeProvisionerDaemon: true,
Pubsub: pubsub, Pubsub: pubsub,
StatsBatcher: batcher, StatsBatcher: batcher,
Logger: &tLogger,
}) })
user := coderdtest.CreateFirstUser(t, client) user := coderdtest.CreateFirstUser(t, client)

View File

@ -8,10 +8,21 @@ import (
"golang.org/x/xerrors" "golang.org/x/xerrors"
) )
type WorkspaceAgentScopeParams struct {
WorkspaceID uuid.UUID
OwnerID uuid.UUID
TemplateID uuid.UUID
VersionID uuid.UUID
}
// WorkspaceAgentScope returns a scope that is the same as ScopeAll but can only // WorkspaceAgentScope returns a scope that is the same as ScopeAll but can only
// affect resources in the allow list. Only a scope is returned as the roles // affect resources in the allow list. Only a scope is returned as the roles
// should come from the workspace owner. // should come from the workspace owner.
func WorkspaceAgentScope(workspaceID, ownerID uuid.UUID) Scope { func WorkspaceAgentScope(params WorkspaceAgentScopeParams) Scope {
if params.WorkspaceID == uuid.Nil || params.OwnerID == uuid.Nil || params.TemplateID == uuid.Nil || params.VersionID == uuid.Nil {
panic("all uuids must be non-nil, this is a developer error")
}
allScope, err := ScopeAll.Expand() allScope, err := ScopeAll.Expand()
if err != nil { if err != nil {
panic("failed to expand scope all, this should never happen") panic("failed to expand scope all, this should never happen")
@ -23,10 +34,13 @@ func WorkspaceAgentScope(workspaceID, ownerID uuid.UUID) Scope {
// and evolving. // and evolving.
Role: allScope.Role, Role: allScope.Role,
// This prevents the agent from being able to access any other resource. // This prevents the agent from being able to access any other resource.
// Include the list of IDs of anything that is required for the
// agent to function.
AllowIDList: []string{ AllowIDList: []string{
workspaceID.String(), params.WorkspaceID.String(),
ownerID.String(), params.TemplateID.String(),
// TODO: Might want to include the template the workspace uses too? params.VersionID.String(),
params.OwnerID.String(),
}, },
} }
} }

View File

@ -1488,7 +1488,7 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques
templateSchedule, err := (*(api.TemplateScheduleStore.Load())).Get(ctx, api.Database, workspace.TemplateID) templateSchedule, err := (*(api.TemplateScheduleStore.Load())).Get(ctx, api.Database, workspace.TemplateID)
// If the template schedule fails to load, just default to bumping without the next transition and log it. // If the template schedule fails to load, just default to bumping without the next transition and log it.
if err != nil { if err != nil {
api.Logger.Warn(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min", api.Logger.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
slog.F("workspace_id", workspace.ID), slog.F("workspace_id", workspace.ID),
slog.F("template_id", workspace.TemplateID), slog.F("template_id", workspace.TemplateID),
slog.Error(err), slog.Error(err),