Skip to content

Commit 8b9dfc9

Browse files
EmyrkEmyrk
committed
chore: remove UpsertCustomRole in favor of Insert + Update
1 parent 76722a7 commit 8b9dfc9

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
@@ -2543,6 +2543,114 @@ func (q *querier) InsertAuditLog(ctx context.Context, arg database.InsertAuditLo
25432543
return insert(q.log, q.auth, rbac.ResourceAuditLog, q.db.InsertAuditLog)(ctx, arg)
25442544
}
25452545

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

3105+
func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) {
3106+
panic("not implemented")
3107+
}
3108+
29973109
func (q *querier) UpdateExternalAuthLink(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) {
29983110
fetch := func(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) {
29993111
return q.db.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{UserID: arg.UserID, ProviderID: arg.ProviderID})
@@ -3656,91 +3768,6 @@ func (q *querier) UpsertApplicationName(ctx context.Context, value string) error
36563768
return q.db.UpsertApplicationName(ctx, value)
36573769
}
36583770

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

‎coderd/database/dbgen/dbgen.go

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

838838
func CustomRole(t testing.TB, db database.Store, seed database.CustomRole) database.CustomRole {
839-
role, err := db.UpsertCustomRole(genCtx, database.UpsertCustomRoleParams{
839+
role, err := db.InsertCustomRole(genCtx, database.InsertCustomRoleParams{
840840
Name: takeFirst(seed.Name, strings.ToLower(testutil.GetRandomName(t))),
841841
DisplayName: testutil.GetRandomName(t),
842842
OrganizationID: seed.OrganizationID,

‎coderd/database/dbmem/dbmem.go

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

6089+
func (q *FakeQuerier) InsertCustomRole(_ context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) {
6090+
err := validateDatabaseType(arg)
6091+
if err != nil {
6092+
return database.CustomRole{}, err
6093+
}
6094+
6095+
q.mutex.RLock()
6096+
defer q.mutex.RUnlock()
6097+
for i := range q.customRoles {
6098+
if strings.EqualFold(q.customRoles[i].Name, arg.Name) &&
6099+
q.customRoles[i].OrganizationID.UUID == arg.OrganizationID.UUID {
6100+
return database.CustomRole{}, errUniqueConstraint
6101+
}
6102+
}
6103+
6104+
role := database.CustomRole{
6105+
ID: uuid.New(),
6106+
Name: arg.Name,
6107+
DisplayName: arg.DisplayName,
6108+
OrganizationID: arg.OrganizationID,
6109+
SitePermissions: arg.SitePermissions,
6110+
OrgPermissions: arg.OrgPermissions,
6111+
UserPermissions: arg.UserPermissions,
6112+
CreatedAt: dbtime.Now(),
6113+
UpdatedAt: dbtime.Now(),
6114+
}
6115+
q.customRoles = append(q.customRoles, role)
6116+
6117+
return role, nil
6118+
}
6119+
60896120
func (q *FakeQuerier) InsertDBCryptKey(_ context.Context, arg database.InsertDBCryptKeyParams) error {
60906121
err := validateDatabaseType(arg)
60916122
if err != nil {
@@ -7456,6 +7487,29 @@ func (q *FakeQuerier) UpdateAPIKeyByID(_ context.Context, arg database.UpdateAPI
74567487
return sql.ErrNoRows
74577488
}
74587489

7490+
func (q *FakeQuerier) UpdateCustomRole(_ context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) {
7491+
err := validateDatabaseType(arg)
7492+
if err != nil {
7493+
return database.CustomRole{}, err
7494+
}
7495+
7496+
q.mutex.RLock()
7497+
defer q.mutex.RUnlock()
7498+
for i := range q.customRoles {
7499+
if strings.EqualFold(q.customRoles[i].Name, arg.Name) &&
7500+
q.customRoles[i].OrganizationID.UUID == arg.OrganizationID.UUID {
7501+
q.customRoles[i].DisplayName = arg.DisplayName
7502+
q.customRoles[i].OrganizationID = arg.OrganizationID
7503+
q.customRoles[i].SitePermissions = arg.SitePermissions
7504+
q.customRoles[i].OrgPermissions = arg.OrgPermissions
7505+
q.customRoles[i].UserPermissions = arg.UserPermissions
7506+
q.customRoles[i].UpdatedAt = dbtime.Now()
7507+
return q.customRoles[i], nil
7508+
}
7509+
}
7510+
return database.CustomRole{}, sql.ErrNoRows
7511+
}
7512+
74597513
func (q *FakeQuerier) UpdateExternalAuthLink(_ context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) {
74607514
if err := validateDatabaseType(arg); err != nil {
74617515
return database.ExternalAuthLink{}, err
@@ -8800,42 +8854,6 @@ func (q *FakeQuerier) UpsertApplicationName(_ context.Context, data string) erro
88008854
return nil
88018855
}
88028856

8803-
func (q *FakeQuerier) UpsertCustomRole(_ context.Context, arg database.UpsertCustomRoleParams) (database.CustomRole, error) {
8804-
err := validateDatabaseType(arg)
8805-
if err != nil {
8806-
return database.CustomRole{}, err
8807-
}
8808-
8809-
q.mutex.RLock()
8810-
defer q.mutex.RUnlock()
8811-
for i := range q.customRoles {
8812-
if strings.EqualFold(q.customRoles[i].Name, arg.Name) {
8813-
q.customRoles[i].DisplayName = arg.DisplayName
8814-
q.customRoles[i].OrganizationID = arg.OrganizationID
8815-
q.customRoles[i].SitePermissions = arg.SitePermissions
8816-
q.customRoles[i].OrgPermissions = arg.OrgPermissions
8817-
q.customRoles[i].UserPermissions = arg.UserPermissions
8818-
q.customRoles[i].UpdatedAt = dbtime.Now()
8819-
return q.customRoles[i], nil
8820-
}
8821-
}
8822-
8823-
role := database.CustomRole{
8824-
ID: uuid.New(),
8825-
Name: arg.Name,
8826-
DisplayName: arg.DisplayName,
8827-
OrganizationID: arg.OrganizationID,
8828-
SitePermissions: arg.SitePermissions,
8829-
OrgPermissions: arg.OrgPermissions,
8830-
UserPermissions: arg.UserPermissions,
8831-
CreatedAt: dbtime.Now(),
8832-
UpdatedAt: dbtime.Now(),
8833-
}
8834-
q.customRoles = append(q.customRoles, role)
8835-
8836-
return role, nil
8837-
}
8838-
88398857
func (q *FakeQuerier) UpsertDefaultProxy(_ context.Context, arg database.UpsertDefaultProxyParams) error {
88408858
q.defaultProxyDisplayName = arg.DisplayName
88418859
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)