Skip to content

Commit b1776c1

Browse files
EmyrkEmyrk
committed
chore: remove UpsertCustomRole in favor of Insert + Update
1 parent 712a1b5 commit b1776c1

File tree

11 files changed

+416
-183
lines changed

11 files changed

+416
-183
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 112 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,6 +2551,114 @@ func (q *querier) InsertAuditLog(ctx context.Context, arg database.InsertAuditLo
25512551
return insert(q.log, q.auth, rbac.ResourceAuditLog, q.db.InsertAuditLog)(ctx, arg)
25522552
}
25532553

2554+
func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) {
2555+
// Org and site role upsert share the same query. So switch the assertion based on the org uuid.
2556+
if arg.OrganizationID.UUID != uuid.Nil {
2557+
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
2558+
return database.CustomRole{}, err
2559+
}
2560+
} else {
2561+
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil {
2562+
return database.CustomRole{}, err
2563+
}
2564+
}
2565+
2566+
if err := q.customRoleCheck(ctx, database.CustomRole{
2567+
Name: arg.Name,
2568+
DisplayName: arg.DisplayName,
2569+
SitePermissions: arg.SitePermissions,
2570+
OrgPermissions: arg.OrgPermissions,
2571+
UserPermissions: arg.UserPermissions,
2572+
CreatedAt: time.Now(),
2573+
UpdatedAt: time.Now(),
2574+
OrganizationID: arg.OrganizationID,
2575+
ID: uuid.New(),
2576+
}); err != nil {
2577+
return database.CustomRole{}, err
2578+
}
2579+
return q.db.InsertCustomRole(ctx, arg)
2580+
}
2581+
2582+
// customRoleCheck will validate a custom role for inserting or updating.
2583+
// If the role is not valid, an error will be returned.
2584+
// - Check custom roles are valid for their resource types + actions
2585+
// - Check the actor can create the custom role
2586+
// - Check the custom role does not grant perms the actor does not have
2587+
// - Prevent negative perms
2588+
// - Prevent roles with site and org permissions.
2589+
func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole) error {
2590+
act, ok := ActorFromContext(ctx)
2591+
if !ok {
2592+
return NoActorError
2593+
}
2594+
2595+
// Org permissions require an org role
2596+
if role.OrganizationID.UUID == uuid.Nil && len(role.OrgPermissions) > 0 {
2597+
return xerrors.Errorf("organization permissions require specifying an organization id")
2598+
}
2599+
2600+
// Org roles can only specify org permissions
2601+
if role.OrganizationID.UUID != uuid.Nil && (len(role.SitePermissions) > 0 || len(role.UserPermissions) > 0) {
2602+
return xerrors.Errorf("organization roles specify site or user permissions")
2603+
}
2604+
2605+
// The rbac.Role has a 'Valid()' function on it that will do a lot
2606+
// of checks.
2607+
rbacRole, err := rolestore.ConvertDBRole(database.CustomRole{
2608+
Name: role.Name,
2609+
DisplayName: role.DisplayName,
2610+
SitePermissions: role.SitePermissions,
2611+
OrgPermissions: role.OrgPermissions,
2612+
UserPermissions: role.UserPermissions,
2613+
OrganizationID: role.OrganizationID,
2614+
})
2615+
if err != nil {
2616+
return xerrors.Errorf("invalid args: %w", err)
2617+
}
2618+
2619+
err = rbacRole.Valid()
2620+
if err != nil {
2621+
return xerrors.Errorf("invalid role: %w", err)
2622+
}
2623+
2624+
if len(rbacRole.Org) > 0 && len(rbacRole.Site) > 0 {
2625+
// This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can
2626+
// do what gets more complicated.
2627+
return xerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time")
2628+
}
2629+
2630+
if len(rbacRole.Org) > 1 {
2631+
// Again to avoid more complexity in our roles
2632+
return xerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time")
2633+
}
2634+
2635+
// Prevent escalation
2636+
for _, sitePerm := range rbacRole.Site {
2637+
err := q.customRoleEscalationCheck(ctx, act, sitePerm, rbac.Object{Type: sitePerm.ResourceType})
2638+
if err != nil {
2639+
return xerrors.Errorf("site permission: %w", err)
2640+
}
2641+
}
2642+
2643+
for orgID, perms := range rbacRole.Org {
2644+
for _, orgPerm := range perms {
2645+
err := q.customRoleEscalationCheck(ctx, act, orgPerm, rbac.Object{OrgID: orgID, Type: orgPerm.ResourceType})
2646+
if err != nil {
2647+
return xerrors.Errorf("org=%q: %w", orgID, err)
2648+
}
2649+
}
2650+
}
2651+
2652+
for _, userPerm := range rbacRole.User {
2653+
err := q.customRoleEscalationCheck(ctx, act, userPerm, rbac.Object{Type: userPerm.ResourceType, Owner: act.ID})
2654+
if err != nil {
2655+
return xerrors.Errorf("user permission: %w", err)
2656+
}
2657+
}
2658+
2659+
return nil
2660+
}
2661+
25542662
func (q *querier) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error {
25552663
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
25562664
return err
@@ -3002,6 +3110,10 @@ func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKe
30023110
return update(q.log, q.auth, fetch, q.db.UpdateAPIKeyByID)(ctx, arg)
30033111
}
30043112

3113+
func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) {
3114+
panic("not implemented")
3115+
}
3116+
30053117
func (q *querier) UpdateExternalAuthLink(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) {
30063118
fetch := func(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) {
30073119
return q.db.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{UserID: arg.UserID, ProviderID: arg.ProviderID})
@@ -3664,91 +3776,6 @@ func (q *querier) UpsertApplicationName(ctx context.Context, value string) error
36643776
return q.db.UpsertApplicationName(ctx, value)
36653777
}
36663778

3667-
// UpsertCustomRole does a series of authz checks to protect custom roles.
3668-
// - Check custom roles are valid for their resource types + actions
3669-
// - Check the actor can create the custom role
3670-
// - Check the custom role does not grant perms the actor does not have
3671-
// - Prevent negative perms
3672-
// - Prevent roles with site and org permissions.
3673-
func (q *querier) UpsertCustomRole(ctx context.Context, arg database.UpsertCustomRoleParams) (database.CustomRole, error) {
3674-
act, ok := ActorFromContext(ctx)
3675-
if !ok {
3676-
return database.CustomRole{}, NoActorError
3677-
}
3678-
3679-
// Org and site role upsert share the same query. So switch the assertion based on the org uuid.
3680-
if arg.OrganizationID.UUID != uuid.Nil {
3681-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
3682-
return database.CustomRole{}, err
3683-
}
3684-
} else {
3685-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil {
3686-
return database.CustomRole{}, err
3687-
}
3688-
}
3689-
3690-
if arg.OrganizationID.UUID == uuid.Nil && len(arg.OrgPermissions) > 0 {
3691-
return database.CustomRole{}, xerrors.Errorf("organization permissions require specifying an organization id")
3692-
}
3693-
3694-
// There is quite a bit of validation we should do here.
3695-
// The rbac.Role has a 'Valid()' function on it that will do a lot
3696-
// of checks.
3697-
rbacRole, err := rolestore.ConvertDBRole(database.CustomRole{
3698-
Name: arg.Name,
3699-
DisplayName: arg.DisplayName,
3700-
SitePermissions: arg.SitePermissions,
3701-
OrgPermissions: arg.OrgPermissions,
3702-
UserPermissions: arg.UserPermissions,
3703-
OrganizationID: arg.OrganizationID,
3704-
})
3705-
if err != nil {
3706-
return database.CustomRole{}, xerrors.Errorf("invalid args: %w", err)
3707-
}
3708-
3709-
err = rbacRole.Valid()
3710-
if err != nil {
3711-
return database.CustomRole{}, xerrors.Errorf("invalid role: %w", err)
3712-
}
3713-
3714-
if len(rbacRole.Org) > 0 && len(rbacRole.Site) > 0 {
3715-
// This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can
3716-
// do what gets more complicated.
3717-
return database.CustomRole{}, xerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time")
3718-
}
3719-
3720-
if len(rbacRole.Org) > 1 {
3721-
// Again to avoid more complexity in our roles
3722-
return database.CustomRole{}, xerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time")
3723-
}
3724-
3725-
// Prevent escalation
3726-
for _, sitePerm := range rbacRole.Site {
3727-
err := q.customRoleEscalationCheck(ctx, act, sitePerm, rbac.Object{Type: sitePerm.ResourceType})
3728-
if err != nil {
3729-
return database.CustomRole{}, xerrors.Errorf("site permission: %w", err)
3730-
}
3731-
}
3732-
3733-
for orgID, perms := range rbacRole.Org {
3734-
for _, orgPerm := range perms {
3735-
err := q.customRoleEscalationCheck(ctx, act, orgPerm, rbac.Object{OrgID: orgID, Type: orgPerm.ResourceType})
3736-
if err != nil {
3737-
return database.CustomRole{}, xerrors.Errorf("org=%q: %w", orgID, err)
3738-
}
3739-
}
3740-
}
3741-
3742-
for _, userPerm := range rbacRole.User {
3743-
err := q.customRoleEscalationCheck(ctx, act, userPerm, rbac.Object{Type: userPerm.ResourceType, Owner: act.ID})
3744-
if err != nil {
3745-
return database.CustomRole{}, xerrors.Errorf("user permission: %w", err)
3746-
}
3747-
}
3748-
3749-
return q.db.UpsertCustomRole(ctx, arg)
3750-
}
3751-
37523779
func (q *querier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error {
37533780
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
37543781
return err

‎coderd/database/dbgen/dbgen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -880,7 +880,7 @@ func OAuth2ProviderAppToken(t testing.TB, db database.Store, seed database.OAuth
880880
}
881881

882882
func CustomRole(t testing.TB, db database.Store, seed database.CustomRole) database.CustomRole {
883-
role, err := db.UpsertCustomRole(genCtx, database.UpsertCustomRoleParams{
883+
role, err := db.InsertCustomRole(genCtx, database.InsertCustomRoleParams{
884884
Name: takeFirst(seed.Name, strings.ToLower(testutil.GetRandomName(t))),
885885
DisplayName: testutil.GetRandomName(t),
886886
OrganizationID: seed.OrganizationID,

‎coderd/database/dbmem/dbmem.go

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6161,6 +6161,37 @@ func (q *FakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAudit
61616161
return alog, nil
61626162
}
61636163

6164+
func (q *FakeQuerier) InsertCustomRole(_ context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) {
6165+
err := validateDatabaseType(arg)
6166+
if err != nil {
6167+
return database.CustomRole{}, err
6168+
}
6169+
6170+
q.mutex.RLock()
6171+
defer q.mutex.RUnlock()
6172+
for i := range q.customRoles {
6173+
if strings.EqualFold(q.customRoles[i].Name, arg.Name) &&
6174+
q.customRoles[i].OrganizationID.UUID == arg.OrganizationID.UUID {
6175+
return database.CustomRole{}, errUniqueConstraint
6176+
}
6177+
}
6178+
6179+
role := database.CustomRole{
6180+
ID: uuid.New(),
6181+
Name: arg.Name,
6182+
DisplayName: arg.DisplayName,
6183+
OrganizationID: arg.OrganizationID,
6184+
SitePermissions: arg.SitePermissions,
6185+
OrgPermissions: arg.OrgPermissions,
6186+
UserPermissions: arg.UserPermissions,
6187+
CreatedAt: dbtime.Now(),
6188+
UpdatedAt: dbtime.Now(),
6189+
}
6190+
q.customRoles = append(q.customRoles, role)
6191+
6192+
return role, nil
6193+
}
6194+
61646195
func (q *FakeQuerier) InsertDBCryptKey(_ context.Context, arg database.InsertDBCryptKeyParams) error {
61656196
err := validateDatabaseType(arg)
61666197
if err != nil {
@@ -7531,6 +7562,29 @@ func (q *FakeQuerier) UpdateAPIKeyByID(_ context.Context, arg database.UpdateAPI
75317562
return sql.ErrNoRows
75327563
}
75337564

7565+
func (q *FakeQuerier) UpdateCustomRole(_ context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) {
7566+
err := validateDatabaseType(arg)
7567+
if err != nil {
7568+
return database.CustomRole{}, err
7569+
}
7570+
7571+
q.mutex.RLock()
7572+
defer q.mutex.RUnlock()
7573+
for i := range q.customRoles {
7574+
if strings.EqualFold(q.customRoles[i].Name, arg.Name) &&
7575+
q.customRoles[i].OrganizationID.UUID == arg.OrganizationID.UUID {
7576+
q.customRoles[i].DisplayName = arg.DisplayName
7577+
q.customRoles[i].OrganizationID = arg.OrganizationID
7578+
q.customRoles[i].SitePermissions = arg.SitePermissions
7579+
q.customRoles[i].OrgPermissions = arg.OrgPermissions
7580+
q.customRoles[i].UserPermissions = arg.UserPermissions
7581+
q.customRoles[i].UpdatedAt = dbtime.Now()
7582+
return q.customRoles[i], nil
7583+
}
7584+
}
7585+
return database.CustomRole{}, sql.ErrNoRows
7586+
}
7587+
75347588
func (q *FakeQuerier) UpdateExternalAuthLink(_ context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) {
75357589
if err := validateDatabaseType(arg); err != nil {
75367590
return database.ExternalAuthLink{}, err
@@ -8875,42 +8929,6 @@ func (q *FakeQuerier) UpsertApplicationName(_ context.Context, data string) erro
88758929
return nil
88768930
}
88778931

8878-
func (q *FakeQuerier) UpsertCustomRole(_ context.Context, arg database.UpsertCustomRoleParams) (database.CustomRole, error) {
8879-
err := validateDatabaseType(arg)
8880-
if err != nil {
8881-
return database.CustomRole{}, err
8882-
}
8883-
8884-
q.mutex.RLock()
8885-
defer q.mutex.RUnlock()
8886-
for i := range q.customRoles {
8887-
if strings.EqualFold(q.customRoles[i].Name, arg.Name) {
8888-
q.customRoles[i].DisplayName = arg.DisplayName
8889-
q.customRoles[i].OrganizationID = arg.OrganizationID
8890-
q.customRoles[i].SitePermissions = arg.SitePermissions
8891-
q.customRoles[i].OrgPermissions = arg.OrgPermissions
8892-
q.customRoles[i].UserPermissions = arg.UserPermissions
8893-
q.customRoles[i].UpdatedAt = dbtime.Now()
8894-
return q.customRoles[i], nil
8895-
}
8896-
}
8897-
8898-
role := database.CustomRole{
8899-
ID: uuid.New(),
8900-
Name: arg.Name,
8901-
DisplayName: arg.DisplayName,
8902-
OrganizationID: arg.OrganizationID,
8903-
SitePermissions: arg.SitePermissions,
8904-
OrgPermissions: arg.OrgPermissions,
8905-
UserPermissions: arg.UserPermissions,
8906-
CreatedAt: dbtime.Now(),
8907-
UpdatedAt: dbtime.Now(),
8908-
}
8909-
q.customRoles = append(q.customRoles, role)
8910-
8911-
return role, nil
8912-
}
8913-
89148932
func (q *FakeQuerier) UpsertDefaultProxy(_ context.Context, arg database.UpsertDefaultProxyParams) error {
89158933
q.defaultProxyDisplayName = arg.DisplayName
89168934
q.defaultProxyIconURL = arg.IconUrl

‎coderd/database/dbmetrics/dbmetrics.go

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

0 commit comments

Comments
 (0)