Skip to content

Commit deadac0

Browse files
aslilacaslilac
authored
fix: fix loading states and permissions checks in organization settings (coder#16465)
1 parent e59c54a commit deadac0

35 files changed

+709
-864
lines changed

‎coderd/rbac/roles.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -297,18 +297,17 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
297297
Identifier: RoleAuditor(),
298298
DisplayName: "Auditor",
299299
Site: Permissions(map[string][]policy.Action{
300-
// Should be able to read all template details, even in orgs they
301-
// are not in.
302-
ResourceTemplate.Type: {policy.ActionRead, policy.ActionViewInsights},
303-
ResourceAuditLog.Type: {policy.ActionRead},
304-
ResourceUser.Type: {policy.ActionRead},
305-
ResourceGroup.Type: {policy.ActionRead},
306-
ResourceGroupMember.Type: {policy.ActionRead},
300+
ResourceAuditLog.Type: {policy.ActionRead},
301+
// Allow auditors to see the resources that audit logs reflect.
302+
ResourceTemplate.Type: {policy.ActionRead, policy.ActionViewInsights},
303+
ResourceUser.Type: {policy.ActionRead},
304+
ResourceGroup.Type: {policy.ActionRead},
305+
ResourceGroupMember.Type: {policy.ActionRead},
306+
ResourceOrganization.Type: {policy.ActionRead},
307+
ResourceOrganizationMember.Type: {policy.ActionRead},
307308
// Allow auditors to query deployment stats and insights.
308309
ResourceDeploymentStats.Type: {policy.ActionRead},
309310
ResourceDeploymentConfig.Type: {policy.ActionRead},
310-
// Org roles are not really used yet, so grant the perm at the site level.
311-
ResourceOrganizationMember.Type: {policy.ActionRead},
312311
}),
313312
Org: map[string][]Permission{},
314313
User: []Permission{},
@@ -325,11 +324,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
325324
// CRUD to provisioner daemons for now.
326325
ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
327326
// Needs to read all organizations since
328-
ResourceOrganization.Type: {policy.ActionRead},
329-
ResourceUser.Type: {policy.ActionRead},
330-
ResourceGroup.Type: {policy.ActionRead},
331-
ResourceGroupMember.Type: {policy.ActionRead},
332-
// Org roles are not really used yet, so grant the perm at the site level.
327+
ResourceUser.Type: {policy.ActionRead},
328+
ResourceGroup.Type: {policy.ActionRead},
329+
ResourceGroupMember.Type: {policy.ActionRead},
330+
ResourceOrganization.Type: {policy.ActionRead},
333331
ResourceOrganizationMember.Type: {policy.ActionRead},
334332
}),
335333
Org: map[string][]Permission{},
@@ -348,10 +346,11 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
348346
policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete,
349347
policy.ActionUpdatePersonal, policy.ActionReadPersonal,
350348
},
349+
ResourceGroup.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
350+
ResourceGroupMember.Type: {policy.ActionRead},
351+
ResourceOrganization.Type: {policy.ActionRead},
351352
// Full perms to manage org members
352353
ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
353-
ResourceGroup.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
354-
ResourceGroupMember.Type: {policy.ActionRead},
355354
// Manage org membership based on OIDC claims
356355
ResourceIdpsyncSettings.Type: {policy.ActionRead, policy.ActionUpdate},
357356
}),

‎coderd/rbac/roles_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ func TestRolePermissions(t *testing.T) {
117117
owner := authSubject{Name: "owner", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleOwner()}}}
118118
templateAdmin := authSubject{Name: "template-admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleTemplateAdmin()}}}
119119
userAdmin := authSubject{Name: "user-admin", Actor: rbac.Subject{ID: userAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleUserAdmin()}}}
120+
auditor := authSubject{Name: "auditor", Actor: rbac.Subject{ID: auditorID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleAuditor()}}}
120121

121122
orgAdmin := authSubject{Name: "org_admin", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgAdmin(orgID)}}}
122123
orgAuditor := authSubject{Name: "org_auditor", Actor: rbac.Subject{ID: auditorID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID), rbac.ScopedRoleOrgAuditor(orgID)}}}
@@ -286,8 +287,8 @@ func TestRolePermissions(t *testing.T) {
286287
Actions: []policy.Action{policy.ActionRead},
287288
Resource: rbac.ResourceOrganization.WithID(orgID).InOrg(orgID),
288289
AuthorizeMap: map[bool][]hasAuthSubjects{
289-
true: {owner, orgAdmin, orgMemberMe, templateAdmin, orgTemplateAdmin, orgAuditor, orgUserAdmin},
290-
false: {setOtherOrg, memberMe, userAdmin},
290+
true: {owner, orgAdmin, orgMemberMe, templateAdmin, orgTemplateAdmin, auditor, orgAuditor, userAdmin, orgUserAdmin},
291+
false: {setOtherOrg, memberMe},
291292
},
292293
},
293294
{

‎site/src/api/queries/organizations.ts

Lines changed: 28 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
import { API } from "api/api";
22
import type {
3-
AuthorizationResponse,
43
CreateOrganizationRequest,
54
GroupSyncSettings,
65
RoleSyncSettings,
76
UpdateOrganizationRequest,
87
} from "api/typesGenerated";
8+
import {
9+
type AnyOrganizationPermissions,
10+
type OrganizationPermissionName,
11+
type OrganizationPermissions,
12+
anyOrganizationPermissionChecks,
13+
organizationPermissionChecks,
14+
} from "modules/management/organizationPermissions";
915
import type { QueryClient } from "react-query";
1016
import { meKey } from "./users";
1117

@@ -197,53 +203,6 @@ export const patchRoleSyncSettings = (
197203
};
198204
};
199205

200-
/**
201-
* Fetch permissions for a single organization.
202-
*
203-
* If the ID is undefined, return a disabled query.
204-
*/
205-
export const organizationPermissions = (organizationId: string | undefined) => {
206-
if (!organizationId) {
207-
return { enabled: false };
208-
}
209-
return {
210-
queryKey: ["organization", organizationId, "permissions"],
211-
queryFn: () =>
212-
// Only request what we use on individual org settings, members, and group
213-
// pages, which at the moment is whether you can edit the members on the
214-
// members page, create roles on the roles page, and create groups on the
215-
// groups page. The edit organization check for the settings page is
216-
// covered by the multi-org query at the moment, and the edit group check
217-
// on the group page is done on the group itself, not the org, so neither
218-
// show up here.
219-
API.checkAuthorization({
220-
checks: {
221-
editMembers: {
222-
object: {
223-
resource_type: "organization_member",
224-
organization_id: organizationId,
225-
},
226-
action: "update",
227-
},
228-
createGroup: {
229-
object: {
230-
resource_type: "group",
231-
organization_id: organizationId,
232-
},
233-
action: "create",
234-
},
235-
assignOrgRole: {
236-
object: {
237-
resource_type: "assign_org_role",
238-
organization_id: organizationId,
239-
},
240-
action: "create",
241-
},
242-
},
243-
}),
244-
};
245-
};
246-
247206
export const provisionerJobQueryKey = (orgId: string) => [
248207
"organization",
249208
orgId,
@@ -276,58 +235,13 @@ export const organizationsPermissions = (
276235
// per sub-link (settings, groups, roles, and members pages) that tells us
277236
// whether to show that page, since we only show them if you can edit (and
278237
// not, at the moment if you can only view).
279-
const checks = (organizationId: string) => ({
280-
editMembers: {
281-
object: {
282-
resource_type: "organization_member",
283-
organization_id: organizationId,
284-
},
285-
action: "update",
286-
},
287-
editGroups: {
288-
object: {
289-
resource_type: "group",
290-
organization_id: organizationId,
291-
},
292-
action: "update",
293-
},
294-
editOrganization: {
295-
object: {
296-
resource_type: "organization",
297-
organization_id: organizationId,
298-
},
299-
action: "update",
300-
},
301-
assignOrgRole: {
302-
object: {
303-
resource_type: "assign_org_role",
304-
organization_id: organizationId,
305-
},
306-
action: "create",
307-
},
308-
viewProvisioners: {
309-
object: {
310-
resource_type: "provisioner_daemon",
311-
organization_id: organizationId,
312-
},
313-
action: "read",
314-
},
315-
viewIdpSyncSettings: {
316-
object: {
317-
resource_type: "idpsync_settings",
318-
organization_id: organizationId,
319-
},
320-
action: "read",
321-
},
322-
});
323238

324239
// The endpoint takes a flat array, so to avoid collisions prepend each
325240
// check with the org ID (the key can be anything we want).
326241
const prefixedChecks = organizationIds.flatMap((orgId) =>
327-
Object.entries(checks(orgId)).map(([key, val]) => [
328-
`${orgId}.${key}`,
329-
val,
330-
]),
242+
Object.entries(organizationPermissionChecks(orgId)).map(
243+
([key, val]) => [`${orgId}.${key}`, val],
244+
),
331245
);
332246

333247
const response = await API.checkAuthorization({
@@ -343,15 +257,30 @@ export const organizationsPermissions = (
343257
if (!acc[orgId]) {
344258
acc[orgId] = {};
345259
}
346-
acc[orgId][perm] = value;
260+
acc[orgId][perm as OrganizationPermissionName] = value;
347261
return acc;
348262
},
349-
{} as Record<string, AuthorizationResponse>,
350-
);
263+
{} as Record<string, Partial<OrganizationPermissions>>,
264+
) as Record<string, OrganizationPermissions>;
351265
},
352266
};
353267
};
354268

269+
export const anyOrganizationPermissionsKey = [
270+
"authorization",
271+
"anyOrganization",
272+
];
273+
274+
export const anyOrganizationPermissions = () => {
275+
return {
276+
queryKey: anyOrganizationPermissionsKey,
277+
queryFn: () =>
278+
API.checkAuthorization({
279+
checks: anyOrganizationPermissionChecks,
280+
}) as Promise<AnyOrganizationPermissions>,
281+
};
282+
};
283+
355284
export const getOrganizationIdpSyncClaimFieldValuesKey = (
356285
organization: string,
357286
field: string,

‎site/src/contexts/auth/permissions.tsx

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ export const checks = {
1616
readWorkspaceProxies: "readWorkspaceProxies",
1717
editWorkspaceProxies: "editWorkspaceProxies",
1818
createOrganization: "createOrganization",
19-
editAnyOrganization: "editAnyOrganization",
2019
viewAnyGroup: "viewAnyGroup",
2120
createGroup: "createGroup",
2221
viewAllLicenses: "viewAllLicenses",
@@ -122,13 +121,6 @@ export const permissionsToCheck = {
122121
},
123122
action: "create",
124123
},
125-
[checks.editAnyOrganization]: {
126-
object: {
127-
resource_type: "organization",
128-
any_org: true,
129-
},
130-
action: "update",
131-
},
132124
[checks.viewAnyGroup]: {
133125
object: {
134126
resource_type: "group",

‎site/src/modules/dashboard/DashboardProvider.tsx

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { appearance } from "api/queries/appearance";
22
import { entitlements } from "api/queries/entitlements";
33
import { experiments } from "api/queries/experiments";
4-
import { organizations } from "api/queries/organizations";
4+
import {
5+
anyOrganizationPermissions,
6+
organizations,
7+
} from "api/queries/organizations";
58
import type {
69
AppearanceConfig,
710
Entitlements,
@@ -11,6 +14,7 @@ import type {
1114
import { ErrorAlert } from "components/Alert/ErrorAlert";
1215
import { Loader } from "components/Loader/Loader";
1316
import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata";
17+
import { canViewAnyOrganization } from "modules/management/organizationPermissions";
1418
import { type FC, type PropsWithChildren, createContext } from "react";
1519
import { useQuery } from "react-query";
1620
import { selectFeatureVisibility } from "./entitlements";
@@ -21,6 +25,7 @@ export interface DashboardValue {
2125
appearance: AppearanceConfig;
2226
organizations: readonly Organization[];
2327
showOrganizations: boolean;
28+
canViewOrganizationSettings: boolean;
2429
}
2530

2631
export const DashboardContext = createContext<DashboardValue | undefined>(
@@ -33,12 +38,16 @@ export const DashboardProvider: FC<PropsWithChildren> = ({ children }) => {
3338
const experimentsQuery = useQuery(experiments(metadata.experiments));
3439
const appearanceQuery = useQuery(appearance(metadata.appearance));
3540
const organizationsQuery = useQuery(organizations());
41+
const anyOrganizationPermissionsQuery = useQuery(
42+
anyOrganizationPermissions(),
43+
);
3644

3745
const error =
3846
entitlementsQuery.error ||
3947
appearanceQuery.error ||
4048
experimentsQuery.error ||
41-
organizationsQuery.error;
49+
organizationsQuery.error ||
50+
anyOrganizationPermissionsQuery.error;
4251

4352
if (error) {
4453
return <ErrorAlert error={error} />;
@@ -48,7 +57,8 @@ export const DashboardProvider: FC<PropsWithChildren> = ({ children }) => {
4857
!entitlementsQuery.data ||
4958
!appearanceQuery.data ||
5059
!experimentsQuery.data ||
51-
!organizationsQuery.data;
60+
!organizationsQuery.data ||
61+
!anyOrganizationPermissionsQuery.data;
5262

5363
if (isLoading) {
5464
return <Loader fullscreen />;
@@ -58,6 +68,7 @@ export const DashboardProvider: FC<PropsWithChildren> = ({ children }) => {
5868
const organizationsEnabled = selectFeatureVisibility(
5969
entitlementsQuery.data,
6070
).multiple_organizations;
71+
const showOrganizations = hasMultipleOrganizations || organizationsEnabled;
6172

6273
return (
6374
<DashboardContext.Provider
@@ -66,7 +77,10 @@ export const DashboardProvider: FC<PropsWithChildren> = ({ children }) => {
6677
experiments: experimentsQuery.data,
6778
appearance: appearanceQuery.data,
6879
organizations: organizationsQuery.data,
69-
showOrganizations: hasMultipleOrganizations || organizationsEnabled,
80+
showOrganizations,
81+
canViewOrganizationSettings:
82+
showOrganizations &&
83+
canViewAnyOrganization(anyOrganizationPermissionsQuery.data),
7084
}}
7185
>
7286
{children}

‎site/src/modules/dashboard/Navbar/Navbar.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,13 @@ export const Navbar: FC = () => {
1212
const { metadata } = useEmbeddedMetadata();
1313
const buildInfoQuery = useQuery(buildInfo(metadata["build-info"]));
1414

15-
const { appearance, showOrganizations } = useDashboard();
15+
const { appearance, canViewOrganizationSettings } = useDashboard();
1616
const { user: me, permissions, signOut } = useAuthenticated();
1717
const featureVisibility = useFeatureVisibility();
1818
const canViewAuditLog =
19-
featureVisibility.audit_log && Boolean(permissions.viewAnyAuditLog);
20-
const canViewDeployment = Boolean(permissions.viewDeploymentValues);
21-
const canViewOrganizations =
22-
Boolean(permissions.editAnyOrganization) && showOrganizations;
19+
featureVisibility.audit_log && permissions.viewAnyAuditLog;
20+
const canViewDeployment = permissions.viewDeploymentValues;
21+
const canViewOrganizations = canViewOrganizationSettings;
2322
const proxyContextValue = useProxy();
2423
const canViewHealth = canViewDeployment;
2524

‎site/src/modules/dashboard/Navbar/UserDropdown/UserDropdownContent.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { Stack } from "components/Stack/Stack";
2222
import { usePopover } from "components/deprecated/Popover/Popover";
2323
import type { FC } from "react";
2424
import { Link } from "react-router-dom";
25+
2526
export const Language = {
2627
accountLabel: "Account",
2728
signOutLabel: "Sign Out",
@@ -129,7 +130,7 @@ export const UserDropdownContent: FC<UserDropdownContentProps> = ({
129130
</a>
130131
</Tooltip>
131132

132-
{Boolean(buildInfo?.deployment_id) && (
133+
{buildInfo?.deployment_id && (
133134
<div
134135
css={css`
135136
font-size: 12px;
@@ -145,11 +146,11 @@ export const UserDropdownContent: FC<UserDropdownContentProps> = ({
145146
text-overflow: ellipsis;
146147
`}
147148
>
148-
{buildInfo?.deployment_id}
149+
{buildInfo.deployment_id}
149150
</div>
150151
</Tooltip>
151152
<CopyButton
152-
text={buildInfo!.deployment_id}
153+
text={buildInfo.deployment_id}
153154
buttonStyles={css`
154155
width: 16px;
155156
height: 16px;

0 commit comments

Comments
 (0)