diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index dd12efc25fcb2..e4fa5013a16ce 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -17,6 +17,10 @@ var ( Type: "template", } + ResourceUser = Object{ + Type: "user", + } + // ResourceUserRole might be expanded later to allow more granular permissions // to modifying roles. For now, this covers all possible roles, so having this permission // allows granting/deleting **ALL** roles. diff --git a/coderd/roles.go b/coderd/roles.go index 5c9f16672dd9b..205e8633b4bbe 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -40,12 +40,12 @@ func (api *api) checkPermissions(rw http.ResponseWriter, r *http.Request) { return } - var params codersdk.UserPermissionCheckRequest + var params codersdk.UserAuthorizationRequest if !httpapi.Read(rw, r, ¶ms) { return } - response := make(codersdk.UserPermissionCheckResponse) + response := make(codersdk.UserAuthorizationResponse) for k, v := range params.Checks { if v.Object.ResourceType == "" { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ diff --git a/coderd/roles_test.go b/coderd/roles_test.go index b4d79ae6760c4..2c2629c461793 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -12,7 +12,7 @@ import ( "github.com/coder/coder/codersdk" ) -func TestPermissionCheck(t *testing.T) { +func TestAuthorization(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) @@ -28,29 +28,29 @@ func TestPermissionCheck(t *testing.T) { myself = "read-myself" myWorkspace = "read-my-workspace" ) - params := map[string]codersdk.UserPermissionCheck{ + params := map[string]codersdk.UserAuthorization{ allUsers: { - Object: codersdk.UserPermissionCheckObject{ + Object: codersdk.UserAuthorizationObject{ ResourceType: "users", }, Action: "read", }, myself: { - Object: codersdk.UserPermissionCheckObject{ + Object: codersdk.UserAuthorizationObject{ ResourceType: "users", OwnerID: "me", }, Action: "read", }, myWorkspace: { - Object: codersdk.UserPermissionCheckObject{ + Object: codersdk.UserAuthorizationObject{ ResourceType: "workspaces", OwnerID: "me", }, Action: "read", }, readOrgWorkspaces: { - Object: codersdk.UserPermissionCheckObject{ + Object: codersdk.UserAuthorizationObject{ ResourceType: "workspaces", OrganizationID: admin.OrganizationID.String(), }, @@ -61,7 +61,7 @@ func TestPermissionCheck(t *testing.T) { testCases := []struct { Name string Client *codersdk.Client - Check codersdk.UserPermissionCheckResponse + Check codersdk.UserAuthorizationResponse }{ { Name: "Admin", @@ -90,7 +90,7 @@ func TestPermissionCheck(t *testing.T) { c := c t.Run(c.Name, func(t *testing.T) { t.Parallel() - resp, err := c.Client.CheckPermissions(context.Background(), codersdk.UserPermissionCheckRequest{Checks: params}) + resp, err := c.Client.CheckPermissions(context.Background(), codersdk.UserAuthorizationRequest{Checks: params}) require.NoError(t, err, "check perms") require.Equal(t, resp, c.Check) }) diff --git a/codersdk/roles.go b/codersdk/roles.go index c67255b435b53..59b52330b5fb1 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -44,7 +44,7 @@ func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]Ro return roles, json.NewDecoder(res.Body).Decode(&roles) } -func (c *Client) CheckPermissions(ctx context.Context, checks UserPermissionCheckRequest) (UserPermissionCheckResponse, error) { +func (c *Client) CheckPermissions(ctx context.Context, checks UserAuthorizationRequest) (UserAuthorizationResponse, error) { res, err := c.request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/authorization", uuidOrMe(Me)), checks) if err != nil { return nil, err @@ -53,6 +53,6 @@ func (c *Client) CheckPermissions(ctx context.Context, checks UserPermissionChec if res.StatusCode != http.StatusOK { return nil, readBodyAsError(res) } - var roles UserPermissionCheckResponse + var roles UserAuthorizationResponse return roles, json.NewDecoder(res.Body).Decode(&roles) } diff --git a/codersdk/users.go b/codersdk/users.go index 63f8683ebdded..3946d485799dc 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -76,23 +76,23 @@ type UserRoles struct { OrganizationRoles map[uuid.UUID][]string `json:"organization_roles"` } -type UserPermissionCheckResponse map[string]bool +type UserAuthorizationResponse map[string]bool -// UserPermissionCheckRequest is a structure instead of a map because +// UserAuthorizationRequest is a structure instead of a map because // go-playground/validate can only validate structs. If you attempt to pass // a map into 'httpapi.Read', you will get an invalid type error. -type UserPermissionCheckRequest struct { +type UserAuthorizationRequest struct { // Checks is a map keyed with an arbitrary string to a permission check. // The key can be any string that is helpful to the caller, and allows // multiple permission checks to be run in a single request. // The key ensures that each permission check has the same key in the // response. - Checks map[string]UserPermissionCheck `json:"checks"` + Checks map[string]UserAuthorization `json:"checks"` } -// UserPermissionCheck is used to check if a user can do a given action +// UserAuthorization is used to check if a user can do a given action // to a given set of objects. -type UserPermissionCheck struct { +type UserAuthorization struct { // Object can represent a "set" of objects, such as: // - All workspaces in an organization // - All workspaces owned by me @@ -103,12 +103,12 @@ type UserPermissionCheck struct { // owned by 'me', try to also add an 'OrganizationID' to the settings. // Omitting the 'OrganizationID' could produce the incorrect value, as // workspaces have both `user` and `organization` owners. - Object UserPermissionCheckObject `json:"object"` + Object UserAuthorizationObject `json:"object"` // Action can be 'create', 'read', 'update', or 'delete' Action string `json:"action"` } -type UserPermissionCheckObject struct { +type UserAuthorizationObject struct { // ResourceType is the name of the resource. // './coderd/rbac/object.go' has the list of valid resource types. ResourceType string `json:"resource_type"` diff --git a/site/src/api/api.test.ts b/site/src/api/api.test.ts index 3936d90b66005..6f477646437c9 100644 --- a/site/src/api/api.test.ts +++ b/site/src/api/api.test.ts @@ -2,12 +2,6 @@ import axios from "axios" import { getApiKey, login, logout } from "./api" import * as TypesGen from "./typesGenerated" -// Mock the axios module so that no real network requests are made, but rather -// we swap in a resolved or rejected value -// -// See: https://jestjs.io/docs/mock-functions#mocking-modules -jest.mock("axios") - describe("api.ts", () => { describe("login", () => { it("should return LoginResponse", async () => { @@ -15,16 +9,13 @@ describe("api.ts", () => { const loginResponse: TypesGen.LoginWithPasswordResponse = { session_token: "abc_123_test", } - const axiosMockPost = jest.fn().mockImplementationOnce(() => { - return Promise.resolve({ data: loginResponse }) - }) - axios.post = axiosMockPost + jest.spyOn(axios, "post").mockResolvedValueOnce({ data: loginResponse }) // when const result = await login("test", "123") // then - expect(axiosMockPost).toHaveBeenCalled() + expect(axios.post).toHaveBeenCalled() expect(result).toStrictEqual(loginResponse) }) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 0be44bde17fde..460f9c447f437 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -76,6 +76,14 @@ export const getAuthMethods = async (): Promise => { return response.data } +export const checkUserPermissions = async ( + userId: string, + params: TypesGen.UserAuthorizationRequest, +): Promise => { + const response = await axios.post(`/api/v2/users/${userId}/authorization`, params) + return response.data +} + export const getApiKey = async (): Promise => { const response = await axios.post("/api/v2/users/me/keys") return response.data diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c8002598ca4dc..6e11d552764c1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -316,13 +316,13 @@ export interface User { } // From codersdk/users.go:95:6 -export interface UserPermissionCheck { - readonly object: UserPermissionCheckObject +export interface UserAuthorization { + readonly object: UserAuthorizationObject readonly action: string } // From codersdk/users.go:111:6 -export interface UserPermissionCheckObject { +export interface UserAuthorizationObject { readonly resource_type: string readonly owner_id?: string readonly organization_id?: string @@ -330,12 +330,12 @@ export interface UserPermissionCheckObject { } // From codersdk/users.go:84:6 -export interface UserPermissionCheckRequest { - readonly checks: Record +export interface UserAuthorizationRequest { + readonly checks: Record } // From codersdk/users.go:79:6 -export type UserPermissionCheckResponse = Record +export type UserAuthorizationResponse = Record // From codersdk/users.go:74:6 export interface UserRoles { diff --git a/site/src/components/Navbar/Navbar.test.tsx b/site/src/components/Navbar/Navbar.test.tsx new file mode 100644 index 0000000000000..25c610de22d5b --- /dev/null +++ b/site/src/components/Navbar/Navbar.test.tsx @@ -0,0 +1,40 @@ +import { screen, waitFor } from "@testing-library/react" +import React from "react" +import * as API from "../../api/api" +import { renderWithAuth } from "../../testHelpers/renderHelpers" +import { checks } from "../../xServices/auth/authXService" +import { Language as AdminDropdownLanguage } from "../AdminDropdown/AdminDropdown" +import { Navbar } from "./Navbar" + +beforeEach(() => { + jest.resetAllMocks() +}) + +describe("Navbar", () => { + describe("when user has permission to read all users", () => { + it("displays the admin menu", async () => { + const checkUserPermissionsSpy = jest.spyOn(API, "checkUserPermissions").mockResolvedValueOnce({ + [checks.readAllUsers]: true, + }) + + renderWithAuth() + + // Wait for the request is done + await waitFor(() => expect(checkUserPermissionsSpy).toBeCalledTimes(1)) + await screen.findByRole("button", { name: AdminDropdownLanguage.menuTitle }) + }) + }) + + describe("when user has NO permission to read all users", () => { + it("does not display the admin menu", async () => { + const checkUserPermissionsSpy = jest.spyOn(API, "checkUserPermissions").mockResolvedValueOnce({ + [checks.readAllUsers]: false, + }) + renderWithAuth() + + // Wait for the request is done + await waitFor(() => expect(checkUserPermissionsSpy).toBeCalledTimes(1)) + expect(screen.queryByRole("button", { name: AdminDropdownLanguage.menuTitle })).not.toBeInTheDocument() + }) + }) +}) diff --git a/site/src/components/Navbar/Navbar.tsx b/site/src/components/Navbar/Navbar.tsx index 0ac64ef7d1269..64ca212dd9a78 100644 --- a/site/src/components/Navbar/Navbar.tsx +++ b/site/src/components/Navbar/Navbar.tsx @@ -1,5 +1,6 @@ -import { useActor } from "@xstate/react" +import { useActor, useSelector } from "@xstate/react" import React, { useContext } from "react" +import { selectPermissions } from "../../xServices/auth/authSelectors" import { XServiceContext } from "../../xServices/StateContext" import { NavbarView } from "../NavbarView/NavbarView" @@ -7,7 +8,11 @@ export const Navbar: React.FC = () => { const xServices = useContext(XServiceContext) const [authState, authSend] = useActor(xServices.authXService) const { me } = authState.context + const permissions = useSelector(xServices.authXService, selectPermissions) + // When we have more options in the admin dropdown we may want to check this + // for more permissions + const displayAdminDropdown = !!permissions?.readAllUsers const onSignOut = () => authSend("SIGN_OUT") - return + return } diff --git a/site/src/components/NavbarView/NavbarView.stories.tsx b/site/src/components/NavbarView/NavbarView.stories.tsx index cddf562152ace..564fe4cf6781c 100644 --- a/site/src/components/NavbarView/NavbarView.stories.tsx +++ b/site/src/components/NavbarView/NavbarView.stories.tsx @@ -14,7 +14,16 @@ const Template: Story = (args: NavbarViewProps) => { return Promise.resolve() }, @@ -22,7 +31,16 @@ ForAdmin.args = { export const ForMember = Template.bind({}) ForMember.args = { - user: { id: "1", username: "CathyCoder", email: "cathy@coder.com", created_at: "dawn" }, + user: { + id: "1", + username: "CathyCoder", + email: "cathy@coder.com", + created_at: "dawn", + status: "active", + organization_ids: [], + roles: [], + }, + displayAdminDropdown: false, onSignOut: () => { return Promise.resolve() }, diff --git a/site/src/components/NavbarView/NavbarView.test.tsx b/site/src/components/NavbarView/NavbarView.test.tsx index c2f44e6ad5d27..13cc85a789036 100644 --- a/site/src/components/NavbarView/NavbarView.test.tsx +++ b/site/src/components/NavbarView/NavbarView.test.tsx @@ -10,7 +10,7 @@ describe("NavbarView", () => { } it("renders content", async () => { // When - render() + render() // Then await screen.findAllByText("Coder", { exact: false }) @@ -24,7 +24,7 @@ describe("NavbarView", () => { } // When - render() + render() // Then // There should be a 'B' avatar! diff --git a/site/src/components/NavbarView/NavbarView.tsx b/site/src/components/NavbarView/NavbarView.tsx index 5f098718cd1e8..b88a372419941 100644 --- a/site/src/components/NavbarView/NavbarView.tsx +++ b/site/src/components/NavbarView/NavbarView.tsx @@ -12,9 +12,10 @@ import { UserDropdown } from "../UserDropdown/UsersDropdown" export interface NavbarViewProps { user?: TypesGen.User onSignOut: () => void + displayAdminDropdown: boolean } -export const NavbarView: React.FC = ({ user, onSignOut }) => { +export const NavbarView: React.FC = ({ user, onSignOut, displayAdminDropdown }) => { const styles = useStyles() return ( ) diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 1f708476f1149..236d372710474 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -1,4 +1,5 @@ import { rest } from "msw" +import { permissionsToCheck } from "../xServices/auth/authXService" import * as M from "./entities" export const handlers = [ @@ -54,6 +55,17 @@ export const handlers = [ rest.get("/api/v2/users/roles", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockSiteRoles)) }), + rest.post("/api/v2/users/:userId/authorization", async (req, res, ctx) => { + const permissions = Object.keys(permissionsToCheck) + const response = permissions.reduce((obj, permission) => { + return { + ...obj, + [permission]: true, + } + }, {}) + + return res(ctx.status(200), ctx.json(response)) + }), // workspaces rest.get("/api/v2/organizations/:organizationId/workspaces/:userName/:workspaceName", (req, res, ctx) => { diff --git a/site/src/xServices/auth/authSelectors.ts b/site/src/xServices/auth/authSelectors.ts index b57ac2ebfb522..85a69984ff0a3 100644 --- a/site/src/xServices/auth/authSelectors.ts +++ b/site/src/xServices/auth/authSelectors.ts @@ -1,6 +1,12 @@ import { State } from "xstate" import { AuthContext, AuthEvent } from "./authXService" -export const selectOrgId = (state: State): string | undefined => { +type AuthState = State + +export const selectOrgId = (state: AuthState): string | undefined => { return state.context.me?.organization_ids[0] } + +export const selectPermissions = (state: AuthState): AuthContext["permissions"] => { + return state.context.permissions +} diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index fcb9e6f2e703c..86e9dd003d4ac 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -7,6 +7,21 @@ export const Language = { successProfileUpdate: "Updated preferences.", } +export const checks = { + readAllUsers: "readAllUsers", +} as const + +export const permissionsToCheck = { + [checks.readAllUsers]: { + object: { + resource_type: "user", + }, + action: "read", + }, +} as const + +type Permissions = Record + export interface AuthContext { getUserError?: Error | unknown getMethodsError?: Error | unknown @@ -14,6 +29,8 @@ export interface AuthContext { updateProfileError?: Error | unknown me?: TypesGen.User methods?: TypesGen.AuthMethods + permissions?: Permissions + checkPermissionsError?: Error | unknown } export type AuthEvent = @@ -50,6 +67,9 @@ export const authMachine = updateProfile: { data: TypesGen.User } + checkPermissions: { + data: TypesGen.UserAuthorizationResponse + } }, }, id: "authState", @@ -88,7 +108,7 @@ export const authMachine = onDone: [ { actions: ["assignMe", "clearGetUserError"], - target: "signedIn", + target: "gettingPermissions", }, ], onError: [ @@ -100,6 +120,26 @@ export const authMachine = }, tags: "loading", }, + gettingPermissions: { + entry: "clearGetPermissionsError", + invoke: { + src: "checkPermissions", + id: "checkPermissions", + onDone: [ + { + actions: ["assignPermissions"], + target: "signedIn", + }, + ], + onError: [ + { + actions: "assignGetPermissionsError", + target: "signedOut", + }, + ], + }, + tags: "loading", + }, gettingMethods: { invoke: { src: "getMethods", @@ -200,6 +240,15 @@ export const authMachine = return API.updateProfile(context.me.id, event.data) }, + checkPermissions: async (context) => { + if (!context.me) { + throw new Error("No current user found") + } + + return API.checkUserPermissions(context.me.id, { + checks: permissionsToCheck, + }) + }, }, actions: { assignMe: assign({ @@ -242,6 +291,17 @@ export const authMachine = clearUpdateProfileError: assign({ updateProfileError: (_) => undefined, }), + assignPermissions: assign({ + // Setting event.data as Permissions to be more stricted. So we know + // what permissions we asked for. + permissions: (_, event) => event.data as Permissions, + }), + assignGetPermissionsError: assign({ + checkPermissionsError: (_, event) => event.data, + }), + clearGetPermissionsError: assign({ + checkPermissionsError: (_) => undefined, + }), }, }, )