Skip to content

Commit 3f56f51

Browse files
EmyrkEmyrk
committed
chore: remove org map perms in sdk
1 parent 5eaf868 commit 3f56f51

File tree

6 files changed

+46
-43
lines changed

6 files changed

+46
-43
lines changed

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -531,12 +531,16 @@ func Role(role rbac.Role) codersdk.Role {
531531
if err != nil {
532532
roleName = role.Name
533533
}
534+
534535
return codersdk.Role{
535-
Name: roleName,
536-
OrganizationID: orgIDStr,
537-
DisplayName: role.DisplayName,
538-
SitePermissions: List(role.Site, Permission),
539-
OrganizationPermissions: Map(role.Org, ListLazy(Permission)),
536+
Name: roleName,
537+
OrganizationID: orgIDStr,
538+
DisplayName: role.DisplayName,
539+
SitePermissions: List(role.Site, Permission),
540+
// This is not perfect. If there are organization permissions in another
541+
// organization, they will be omitted. This should not be allowed, so
542+
// should never happen.
543+
OrganizationPermissions: List(role.Org[orgIDStr], Permission),
540544
UserPermissions: List(role.User, Permission),
541545
}
542546
}
@@ -550,11 +554,18 @@ func Permission(permission rbac.Permission) codersdk.Permission {
550554
}
551555

552556
func RoleToRBAC(role codersdk.Role) rbac.Role {
557+
orgPerms := map[string][]rbac.Permission{}
558+
if role.OrganizationID != "" {
559+
orgPerms = map[string][]rbac.Permission{
560+
role.OrganizationID: List(role.OrganizationPermissions, PermissionToRBAC),
561+
}
562+
}
563+
553564
return rbac.Role{
554565
Name: rbac.RoleName(role.Name, role.OrganizationID),
555566
DisplayName: role.DisplayName,
556567
Site: List(role.SitePermissions, PermissionToRBAC),
557-
Org: Map(role.OrganizationPermissions, ListLazy(PermissionToRBAC)),
568+
Org: orgPerms,
558569
User: List(role.UserPermissions, PermissionToRBAC),
559570
}
560571
}

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,10 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
614614
return xerrors.Errorf("role %q has invalid uuid for org: %w", r, err)
615615
}
616616

617+
if orgID == nil {
618+
return xerrors.Errorf("should never happen, orgID is nil, but trying to assign an organization role")
619+
}
620+
617621
if roleOrgID != *orgID {
618622
return xerrors.Errorf("attempted to assign role from a different org, role %q to %q", r, orgID.String())
619623
}

‎coderd/roles.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,6 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) {
5555
return
5656
}
5757

58-
if err := httpapi.NameValid(req.Name); err != nil {
59-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
60-
Message: "Invalid role name",
61-
Detail: err.Error(),
62-
})
63-
return
64-
}
65-
6658
updated, ok := handler.PatchOrganizationRole(ctx, api.Database, rw, organization.ID, req)
6759
if !ok {
6860
return

‎codersdk/roles.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ type Role struct {
3939
OrganizationID string `json:"organization_id" table:"organization_id" format:"uuid"`
4040
DisplayName string `json:"display_name" table:"display_name"`
4141
SitePermissions []Permission `json:"site_permissions" table:"site_permissions"`
42-
// map[<org_id>] -> Permissions
43-
OrganizationPermissions map[string][]Permission `json:"organization_permissions" table:"org_permissions"`
44-
UserPermissions []Permission `json:"user_permissions" table:"user_permissions"`
42+
// OrganizationPermissions are specific for the organization in the field 'OrganizationID' above.
43+
OrganizationPermissions []Permission `json:"organization_permissions" table:"org_permissions"`
44+
UserPermissions []Permission `json:"user_permissions" table:"user_permissions"`
4545
}
4646

4747
// FullName returns the role name scoped to the organization ID. This is useful if

‎enterprise/coderd/roles.go

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,15 @@ type enterpriseCustomRoleHandler struct {
2121
func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, db database.Store, rw http.ResponseWriter, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) {
2222
if !h.Enabled {
2323
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
24-
Message: "Custom roles is not enabled",
24+
Message: "Custom roles are not enabled",
25+
})
26+
return codersdk.Role{}, false
27+
}
28+
29+
if err := httpapi.NameValid(role.Name); err != nil {
30+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
31+
Message: "Invalid role name",
32+
Detail: err.Error(),
2533
})
2634
return codersdk.Role{}, false
2735
}
@@ -43,25 +51,14 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context,
4351
return codersdk.Role{}, false
4452
}
4553

46-
if len(role.OrganizationPermissions) > 1 {
54+
if role.OrganizationID != orgID.String() {
4755
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
48-
Message: "Invalid request, Only 1 organization can be assigned permissions",
49-
Detail: "roles can only contain 1 organization",
56+
Message: "Invalid request, organization in role and url must match",
57+
Detail: fmt.Sprintf("role org %q does not match URL %q", role.OrganizationID, orgID.String()),
5058
})
5159
return codersdk.Role{}, false
5260
}
5361

54-
if len(role.OrganizationPermissions) == 1 {
55-
_, exists := role.OrganizationPermissions[orgID.String()]
56-
if !exists {
57-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
58-
Message: fmt.Sprintf("Invalid request, expected permissions for only the organization %q", orgID.String()),
59-
Detail: fmt.Sprintf("only org id %s allowed", orgID.String()),
60-
})
61-
return codersdk.Role{}, false
62-
}
63-
}
64-
6562
// Make sure all permissions inputted are valid according to our policy.
6663
rbacRole := db2sdk.RoleToRBAC(role)
6764
args, err := rolestore.ConvertRoleToDB(rbacRole)

‎enterprise/coderd/roles_test.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,16 @@ func TestCustomOrganizationRole(t *testing.T) {
2020
t.Parallel()
2121
templateAdminCustom := func(orgID uuid.UUID) codersdk.Role {
2222
return codersdk.Role{
23-
Name: "test-role",
24-
DisplayName: "Testing Purposes",
23+
Name: "test-role",
24+
DisplayName: "Testing Purposes",
25+
OrganizationID: orgID.String(),
2526
// Basically creating a template admin manually
2627
SitePermissions: nil,
27-
OrganizationPermissions: map[string][]codersdk.Permission{
28-
orgID.String(): codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
29-
codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead, codersdk.ActionUpdate, codersdk.ActionViewInsights},
30-
codersdk.ResourceFile: {codersdk.ActionCreate, codersdk.ActionRead},
31-
codersdk.ResourceWorkspace: {codersdk.ActionRead},
32-
}),
33-
},
28+
OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
29+
codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead, codersdk.ActionUpdate, codersdk.ActionViewInsights},
30+
codersdk.ResourceFile: {codersdk.ActionCreate, codersdk.ActionRead},
31+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
32+
}),
3433
UserPermissions: nil,
3534
}
3635
}
@@ -84,7 +83,7 @@ func TestCustomOrganizationRole(t *testing.T) {
8483
}), "role missing from org role list")
8584

8685
require.Len(t, foundRole.SitePermissions, 0)
87-
require.Len(t, foundRole.OrganizationPermissions[first.OrganizationID.String()], 7)
86+
require.Len(t, foundRole.OrganizationPermissions, 7)
8887
require.Len(t, foundRole.UserPermissions, 0)
8988
})
9089

@@ -122,7 +121,7 @@ func TestCustomOrganizationRole(t *testing.T) {
122121

123122
// Verify functionality is lost
124123
_, err = owner.PatchOrganizationRole(ctx, first.OrganizationID, templateAdminCustom(first.OrganizationID))
125-
require.ErrorContains(t, err, "roles is not enabled")
124+
require.ErrorContains(t, err, "roles are not enabled")
126125

127126
// Assign the custom template admin role
128127
tmplAdmin, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, role.FullName())

0 commit comments

Comments
 (0)