Skip to content

Commit 992c845

Browse files
EmyrkEmyrk
committed
chore: static custom role assignment (#13297)
For now, only owners can assign custom roles
1 parent 9305177 commit 992c845

File tree

12 files changed

+171
-48
lines changed

12 files changed

+171
-48
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
597597
}
598598

599599
grantedRoles := append(added, removed...)
600-
notBuiltInRoles := make([]string, 0)
600+
customRoles := make([]string, 0)
601601
// Validate that the roles being assigned are valid.
602602
for _, r := range grantedRoles {
603603
_, isOrgRole := rbac.IsOrgRole(r)
@@ -610,25 +610,25 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
610610

611611
// All roles should be valid roles
612612
if _, err := rbac.RoleByName(r); err != nil {
613-
notBuiltInRoles = append(notBuiltInRoles, r)
613+
customRoles = append(customRoles, r)
614614
}
615615
}
616616

617-
notBuiltInRolesMap := make(map[string]struct{}, len(notBuiltInRoles))
618-
for _, r := range notBuiltInRoles {
619-
notBuiltInRolesMap[r] = struct{}{}
617+
customRolesMap := make(map[string]struct{}, len(customRoles))
618+
for _, r := range customRoles {
619+
customRolesMap[r] = struct{}{}
620620
}
621621

622-
if len(notBuiltInRoles) > 0 {
623-
customRoles, err := q.CustomRolesByName(ctx, notBuiltInRoles)
622+
if len(customRoles) > 0 {
623+
customRoles, err := q.CustomRolesByName(ctx, customRoles)
624624
if err != nil {
625625
return xerrors.Errorf("fetching custom roles: %w", err)
626626
}
627627

628628
// If the lists are not identical, then have a problem, as some roles
629629
// provided do no exist.
630-
if len(customRoles) != len(notBuiltInRoles) {
631-
for _, role := range notBuiltInRoles {
630+
if len(customRoles) != len(customRoles) {
631+
for _, role := range customRoles {
632632
// Stop at the first one found. We could make a better error that
633633
// returns them all, but then someone could pass in a large list to make us do
634634
// a lot of loop iterations.
@@ -654,7 +654,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
654654
}
655655

656656
for _, roleName := range grantedRoles {
657-
if _, isCustom := notBuiltInRolesMap[roleName]; isCustom {
657+
if _, isCustom := customRolesMap[roleName]; isCustom {
658658
// For now, use a constant name so our static assign map still works.
659659
roleName = rbac.CustomSiteRole()
660660
}
@@ -750,7 +750,7 @@ func (q *querier) customRoleEscalationCheck(ctx context.Context, actor rbac.Subj
750750

751751
if perm.Action == policy.WildcardSymbol || perm.ResourceType == policy.WildcardSymbol {
752752
// It is possible to check for supersets with wildcards, but wildcards can also
753-
// include resources and actions that do not exist. Custom roles should only be allowed
753+
// include resources and actions that do not exist today. Custom roles should only be allowed
754754
// to include permissions for existing resources.
755755
return xerrors.Errorf("invalid permission for action=%q type=%q, no wildcard symbols", perm.Action, perm.ResourceType)
756756
}

‎coderd/database/dbmem/dbmem.go

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3168,6 +3168,30 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
31683168
GROUP BY
31693169
start_time, user_id, slug, display_name, icon
31703170
),
3171+
-- Analyze the users unique app usage across all templates. Count
3172+
-- usage across consecutive intervals as continuous usage.
3173+
times_used AS (
3174+
SELECT DISTINCT ON (user_id, slug, display_name, icon, uniq)
3175+
slug,
3176+
display_name,
3177+
icon,
3178+
-- Turn start_time into a unique identifier that identifies a users
3179+
-- continuous app usage. The value of uniq is otherwise garbage.
3180+
--
3181+
-- Since we're aggregating per user app usage across templates,
3182+
-- there can be duplicate start_times. To handle this, we use the
3183+
-- dense_rank() function, otherwise row_number() would suffice.
3184+
start_time - (
3185+
dense_rank() OVER (
3186+
PARTITION BY
3187+
user_id, slug, display_name, icon
3188+
ORDER BY
3189+
start_time
3190+
) * '30 minutes'::interval
3191+
) AS uniq
3192+
FROM
3193+
template_usage_stats_with_apps
3194+
),
31713195
*/
31723196

31733197
// Due to query optimizations, this logic is somewhat inverted from
@@ -3179,12 +3203,19 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
31793203
DisplayName string
31803204
Icon string
31813205
}
3206+
type appTimesUsedGroupBy struct {
3207+
UserID uuid.UUID
3208+
Slug string
3209+
DisplayName string
3210+
Icon string
3211+
}
31823212
type appInsightsRow struct {
31833213
appInsightsGroupBy
31843214
TemplateIDs []uuid.UUID
31853215
AppUsageMins int64
31863216
}
31873217
appInsightRows := make(map[appInsightsGroupBy]appInsightsRow)
3218+
appTimesUsedRows := make(map[appTimesUsedGroupBy]map[time.Time]struct{})
31883219
// FROM
31893220
for _, stat := range q.templateUsageStats {
31903221
// WHERE
@@ -3220,9 +3251,42 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
32203251
row.TemplateIDs = append(row.TemplateIDs, stat.TemplateID)
32213252
row.AppUsageMins = least(row.AppUsageMins+appUsage, 30)
32223253
appInsightRows[key] = row
3254+
3255+
// Prepare to do times_used calculation, distinct start times.
3256+
timesUsedKey := appTimesUsedGroupBy{
3257+
UserID: stat.UserID,
3258+
Slug: slug,
3259+
DisplayName: app.DisplayName,
3260+
Icon: app.Icon,
3261+
}
3262+
if appTimesUsedRows[timesUsedKey] == nil {
3263+
appTimesUsedRows[timesUsedKey] = make(map[time.Time]struct{})
3264+
}
3265+
// This assigns a distinct time, so we don't need to
3266+
// dense_rank() later on, we can simply do row_number().
3267+
appTimesUsedRows[timesUsedKey][stat.StartTime] = struct{}{}
32233268
}
32243269
}
32253270

3271+
appTimesUsedTempRows := make(map[appTimesUsedGroupBy][]time.Time)
3272+
for key, times := range appTimesUsedRows {
3273+
for t := range times {
3274+
appTimesUsedTempRows[key] = append(appTimesUsedTempRows[key], t)
3275+
}
3276+
}
3277+
for _, times := range appTimesUsedTempRows {
3278+
slices.SortFunc(times, func(a, b time.Time) int {
3279+
return int(a.Sub(b))
3280+
})
3281+
}
3282+
for key, times := range appTimesUsedTempRows {
3283+
uniq := make(map[time.Time]struct{})
3284+
for i, t := range times {
3285+
uniq[t.Add(-(30 * time.Minute * time.Duration(i)))] = struct{}{}
3286+
}
3287+
appTimesUsedRows[key] = uniq
3288+
}
3289+
32263290
/*
32273291
-- Even though we allow identical apps to be aggregated across
32283292
-- templates, we still want to be able to report which templates
@@ -3307,14 +3371,20 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
33073371

33083372
var rows []database.GetTemplateAppInsightsRow
33093373
for key, gr := range groupedRows {
3310-
rows = append(rows, database.GetTemplateAppInsightsRow{
3374+
row := database.GetTemplateAppInsightsRow{
33113375
TemplateIDs: templateRows[key].TemplateIDs,
33123376
ActiveUsers: int64(len(uniqueSortedUUIDs(gr.ActiveUserIDs))),
33133377
Slug: key.Slug,
33143378
DisplayName: key.DisplayName,
33153379
Icon: key.Icon,
33163380
UsageSeconds: gr.UsageSeconds,
3317-
})
3381+
}
3382+
for tuk, uniq := range appTimesUsedRows {
3383+
if key.Slug == tuk.Slug && key.DisplayName == tuk.DisplayName && key.Icon == tuk.Icon {
3384+
row.TimesUsed += int64(len(uniq))
3385+
}
3386+
}
3387+
rows = append(rows, row)
33183388
}
33193389

33203390
// NOTE(mafredri): Add sorting if we decide on how to handle PostgreSQL collations.

‎coderd/database/dump.sql

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000209_custom_roles.up.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ CREATE TABLE custom_roles (
1818

1919
-- extra convenience meta data.
2020
created_at timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP,
21-
last_updated timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP
21+
updated_at timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP
2222
);
2323

2424
-- Ensure no case variants of the same roles

‎coderd/database/migrations/testdata/fixtures/000209_custom_roles.up.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ INSERT INTO
66
org_permissions,
77
user_permissions,
88
created_at,
9-
last_updated
9+
updated_at
1010
)
1111
VALUES
1212
(

‎coderd/database/models.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 46 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/roles.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ INSERT INTO
1818
org_permissions,
1919
user_permissions,
2020
created_at,
21-
last_updated
21+
updated_at
2222
)
2323
VALUES (
2424
-- Always force lowercase names
@@ -36,6 +36,6 @@ ON CONFLICT (name)
3636
site_permissions = @site_permissions,
3737
org_permissions = @org_permissions,
3838
user_permissions = @user_permissions,
39-
last_updated = now()
39+
updated_at = now()
4040
RETURNING *
4141
;

‎coderd/rbac/object_gen.go

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/policy/policy.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ var RBACPermissions = map[string]PermissionDefinition{
209209
Actions: map[Action]ActionDefinition{
210210
ActionAssign: actDef("ability to assign roles"),
211211
ActionRead: actDef("view what roles are assignable"),
212-
ActionDelete: actDef("ability to delete roles"),
212+
ActionDelete: actDef("ability to unassign roles"),
213+
ActionCreate: actDef("ability to create/delete/edit custom roles"),
213214
},
214215
},
215216
"assign_org_role": {

0 commit comments

Comments
 (0)