feat: Add permissions for links (#1407)

This commit is contained in:
Bruno Quaresma
2022-05-13 14:25:57 +00:00
committed by GitHub
parent 64a8b4ac47
commit e6168ba238
16 changed files with 192 additions and 47 deletions
+4
View File
@@ -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.
+2 -2
View File
@@ -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, &params) {
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{
+8 -8
View File
@@ -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)
})
+2 -2
View File
@@ -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)
}
+8 -8
View File
@@ -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"`
+2 -11
View File
@@ -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)
})
+8
View File
@@ -76,6 +76,14 @@ export const getAuthMethods = async (): Promise<TypesGen.AuthMethods> => {
return response.data
}
export const checkUserPermissions = async (
userId: string,
params: TypesGen.UserAuthorizationRequest,
): Promise<TypesGen.UserAuthorizationResponse> => {
const response = await axios.post<TypesGen.UserAuthorizationResponse>(`/api/v2/users/${userId}/authorization`, params)
return response.data
}
export const getApiKey = async (): Promise<TypesGen.GenerateAPIKeyResponse> => {
const response = await axios.post<TypesGen.GenerateAPIKeyResponse>("/api/v2/users/me/keys")
return response.data
+6 -6
View File
@@ -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<string, UserPermissionCheck>
export interface UserAuthorizationRequest {
readonly checks: Record<string, UserAuthorization>
}
// From codersdk/users.go:79:6
export type UserPermissionCheckResponse = Record<string, boolean>
export type UserAuthorizationResponse = Record<string, boolean>
// From codersdk/users.go:74:6
export interface UserRoles {
@@ -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(<Navbar />)
// 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(<Navbar />)
// Wait for the request is done
await waitFor(() => expect(checkUserPermissionsSpy).toBeCalledTimes(1))
expect(screen.queryByRole("button", { name: AdminDropdownLanguage.menuTitle })).not.toBeInTheDocument()
})
})
})
+7 -2
View File
@@ -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 <NavbarView user={me} onSignOut={onSignOut} />
return <NavbarView user={me} onSignOut={onSignOut} displayAdminDropdown={displayAdminDropdown} />
}
@@ -14,7 +14,16 @@ const Template: Story<NavbarViewProps> = (args: NavbarViewProps) => <NavbarView
export const ForAdmin = Template.bind({})
ForAdmin.args = {
user: { id: "1", username: "Administrator", email: "admin@coder.com", created_at: "dawn" },
user: {
id: "1",
username: "Administrator",
email: "admin@coder.com",
created_at: "dawn",
status: "active",
organization_ids: [],
roles: [],
},
displayAdminDropdown: true,
onSignOut: () => {
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()
},
@@ -10,7 +10,7 @@ describe("NavbarView", () => {
}
it("renders content", async () => {
// When
render(<NavbarView user={MockUser} onSignOut={noop} />)
render(<NavbarView user={MockUser} onSignOut={noop} displayAdminDropdown />)
// Then
await screen.findAllByText("Coder", { exact: false })
@@ -24,7 +24,7 @@ describe("NavbarView", () => {
}
// When
render(<NavbarView user={mockUser} onSignOut={noop} />)
render(<NavbarView user={mockUser} onSignOut={noop} displayAdminDropdown />)
// Then
// There should be a 'B' avatar!
@@ -12,9 +12,10 @@ import { UserDropdown } from "../UserDropdown/UsersDropdown"
export interface NavbarViewProps {
user?: TypesGen.User
onSignOut: () => void
displayAdminDropdown: boolean
}
export const NavbarView: React.FC<NavbarViewProps> = ({ user, onSignOut }) => {
export const NavbarView: React.FC<NavbarViewProps> = ({ user, onSignOut, displayAdminDropdown }) => {
const styles = useStyles()
return (
<nav className={styles.root}>
@@ -31,7 +32,7 @@ export const NavbarView: React.FC<NavbarViewProps> = ({ user, onSignOut }) => {
</ListItem>
</List>
<div className={styles.fullWidth} />
{user && user.email === "admin@coder.com" && <AdminDropdown />}
{displayAdminDropdown && <AdminDropdown />}
<div className={styles.fixed}>{user && <UserDropdown user={user} onSignOut={onSignOut} />}</div>
</nav>
)
+12
View File
@@ -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) => {
+7 -1
View File
@@ -1,6 +1,12 @@
import { State } from "xstate"
import { AuthContext, AuthEvent } from "./authXService"
export const selectOrgId = (state: State<AuthContext, AuthEvent>): string | undefined => {
type AuthState = State<AuthContext, AuthEvent>
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
}
+61 -1
View File
@@ -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<keyof typeof permissionsToCheck, boolean>
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,
}),
},
},
)