From 89810158549866b35097da4cfe64130c0df1553a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 26 May 2022 15:07:32 -0500 Subject: [PATCH 1/2] feat: last rbac routes - move create organization to /organizations. --- coderd/coderd.go | 2 +- coderd/coderd_test.go | 20 +++++++--- coderd/database/modelmethods.go | 4 ++ coderd/organizations.go | 71 +++++++++++++++++++++++++++++++++ coderd/organizations_test.go | 6 +-- coderd/roles_test.go | 2 +- coderd/templateversions.go | 10 +++++ coderd/users.go | 65 ------------------------------ coderd/users_test.go | 2 +- coderd/workspaces.go | 14 ++++++- coderd/workspaces_test.go | 4 +- codersdk/users.go | 4 +- 12 files changed, 121 insertions(+), 83 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index ed8cba833ccf1..85b790646aad8 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -145,6 +145,7 @@ func New(options *Options) *API { authRolesMiddleware, ) r.Get("/", api.organization) + r.Post("/", api.postOrganizations) r.Post("/templateversions", api.postTemplateVersionsByOrganization) r.Route("/templates", func(r chi.Router) { r.Post("/", api.postTemplateByOrganization) @@ -250,7 +251,6 @@ func New(options *Options) *API { r.Post("/keys", api.postAPIKey) r.Route("/organizations", func(r chi.Router) { - r.Post("/", api.postOrganizationsByUser) r.Get("/", api.organizationsByUser) r.Get("/{organizationname}", api.organizationByUserAndName) }) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index 9b1e83a252ee6..084aa7db032f2 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -138,12 +138,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/workspaceagents/{workspaceagent}/pty": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true}, - // TODO: @emyrk these need to be fixed by adding authorize calls - "POST:/api/v2/organizations/{organization}/workspaces": {NoAuthorize: true}, - "POST:/api/v2/users/{user}/organizations": {NoAuthorize: true}, - "GET:/api/v2/workspaces/{workspace}/watch": {NoAuthorize: true}, - "POST:/api/v2/organizations/{organization}/templateversions": {NoAuthorize: true}, - // These endpoints have more assertions. This is good, add more endpoints to assert if you can! "GET:/api/v2/organizations/{organization}": {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)}, "GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization}, @@ -285,11 +279,25 @@ func TestAuthorizeAllEndpoints(t *testing.T) { AssertAction: rbac.ActionRead, AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()), }, + "POST:/api/v2/organizations/{organization}/workspaces": { + AssertAction: rbac.ActionCreate, + // No ID when creating + AssertObject: workspaceRBACObj.WithID(""), + }, + "GET:/api/v2/workspaces/{workspace}/watch": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "POST:/api/v2/users/{user}/organizations/": { + AssertAction: rbac.ActionCreate, + AssertObject: rbac.ResourceOrganization, + }, // These endpoints need payloads to get to the auth part. Payloads will be required "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, "PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true}, "POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, } for k, v := range assertRoute { diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 900bfa1940acf..4d9534f00ef15 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -26,3 +26,7 @@ func (o Organization) RBACObject() rbac.Object { func (d ProvisionerDaemon) RBACObject() rbac.Object { return rbac.ResourceProvisionerDaemon.WithID(d.ID.String()) } + +func (f File) RBACObject() rbac.Object { + return rbac.ResourceFile.WithID(f.Hash).WithOwner(f.CreatedBy.String()) +} diff --git a/coderd/organizations.go b/coderd/organizations.go index a5e3a958817dc..9e15b8511852e 100644 --- a/coderd/organizations.go +++ b/coderd/organizations.go @@ -1,8 +1,14 @@ package coderd import ( + "database/sql" + "errors" + "fmt" "net/http" + "github.com/google/uuid" + "golang.org/x/xerrors" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -22,6 +28,71 @@ func (api *API) organization(rw http.ResponseWriter, r *http.Request) { httpapi.Write(rw, http.StatusOK, convertOrganization(organization)) } +func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { + apiKey := httpmw.APIKey(r) + // Create organization uses the organization resource without an OrgID. + // This means you need the site wide permission to make a new organization. + if !api.Authorize(rw, r, rbac.ActionCreate, + rbac.ResourceOrganization) { + return + } + + var req codersdk.CreateOrganizationRequest + if !httpapi.Read(rw, r, &req) { + return + } + + _, err := api.Database.GetOrganizationByName(r.Context(), req.Name) + if err == nil { + httpapi.Write(rw, http.StatusConflict, httpapi.Response{ + Message: "organization already exists with that name", + }) + return + } + if !errors.Is(err, sql.ErrNoRows) { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("get organization: %s", err.Error()), + }) + return + } + + var organization database.Organization + err = api.Database.InTx(func(db database.Store) error { + organization, err = api.Database.InsertOrganization(r.Context(), database.InsertOrganizationParams{ + ID: uuid.New(), + Name: req.Name, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), + }) + if err != nil { + return xerrors.Errorf("create organization: %w", err) + } + _, err = api.Database.InsertOrganizationMember(r.Context(), database.InsertOrganizationMemberParams{ + OrganizationID: organization.ID, + UserID: apiKey.UserID, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), + Roles: []string{ + // Also assign member role incase they get demoted from admin + rbac.RoleOrgMember(organization.ID), + rbac.RoleOrgAdmin(organization.ID), + }, + }) + if err != nil { + return xerrors.Errorf("create organization member: %w", err) + } + return nil + }) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: err.Error(), + }) + return + } + + httpapi.Write(rw, http.StatusCreated, convertOrganization(organization)) +} + // convertOrganization consumes the database representation and outputs an API friendly representation. func convertOrganization(organization database.Organization) codersdk.Organization { return codersdk.Organization{ diff --git a/coderd/organizations_test.go b/coderd/organizations_test.go index 01b5822a15611..adc35c08d8908 100644 --- a/coderd/organizations_test.go +++ b/coderd/organizations_test.go @@ -38,7 +38,7 @@ func TestOrganizationByUserAndName(t *testing.T) { client := coderdtest.New(t, nil) first := coderdtest.CreateFirstUser(t, client) other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) - org, err := client.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + org, err := client.CreateOrganization(context.Background(), codersdk.CreateOrganizationRequest{ Name: "another", }) require.NoError(t, err) @@ -67,7 +67,7 @@ func TestPostOrganizationsByUser(t *testing.T) { user := coderdtest.CreateFirstUser(t, client) org, err := client.Organization(context.Background(), user.OrganizationID) require.NoError(t, err) - _, err = client.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + _, err = client.CreateOrganization(context.Background(), codersdk.CreateOrganizationRequest{ Name: org.Name, }) var apiErr *codersdk.Error @@ -79,7 +79,7 @@ func TestPostOrganizationsByUser(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) _ = coderdtest.CreateFirstUser(t, client) - _, err := client.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + _, err := client.CreateOrganization(context.Background(), codersdk.CreateOrganizationRequest{ Name: "new", }) require.NoError(t, err) diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 87b994949de31..4820b76567aa4 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -107,7 +107,7 @@ func TestListRoles(t *testing.T) { member := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) orgAdmin := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleOrgAdmin(admin.OrganizationID)) - otherOrg, err := client.CreateOrganization(ctx, admin.UserID.String(), codersdk.CreateOrganizationRequest{ + otherOrg, err := client.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ Name: "other", }) require.NoError(t, err, "create org") diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 0c1c1d3f7de83..1e3ebc390b82b 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -310,6 +310,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht if !httpapi.Read(rw, r, &req) { return } + if req.TemplateID != uuid.Nil { _, err := api.Database.GetTemplateByID(r.Context(), req.TemplateID) if errors.Is(err, sql.ErrNoRows) { @@ -340,6 +341,15 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht return } + // Making a new template version is the same permission as creating a new template. + if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) { + return + } + + if !api.Authorize(rw, r, rbac.ActionRead, file) { + return + } + var templateVersion database.TemplateVersion var provisionerJob database.ProvisionerJob err = api.Database.InTx(func(db database.Store) error { diff --git a/coderd/users.go b/coderd/users.go index 8e927a9466d7b..02d46655d4f54 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -539,71 +539,6 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques httpapi.Write(rw, http.StatusOK, convertOrganization(organization)) } -func (api *API) postOrganizationsByUser(rw http.ResponseWriter, r *http.Request) { - user := httpmw.UserParam(r) - var req codersdk.CreateOrganizationRequest - if !httpapi.Read(rw, r, &req) { - return - } - - // Create organization uses the organization resource without an OrgID. - // This means you need the site wide permission to make a new organization. - if !api.Authorize(rw, r, rbac.ActionCreate, - rbac.ResourceOrganization) { - return - } - - _, err := api.Database.GetOrganizationByName(r.Context(), req.Name) - if err == nil { - httpapi.Write(rw, http.StatusConflict, httpapi.Response{ - Message: "organization already exists with that name", - }) - return - } - if !errors.Is(err, sql.ErrNoRows) { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("get organization: %s", err.Error()), - }) - return - } - - var organization database.Organization - err = api.Database.InTx(func(db database.Store) error { - organization, err = api.Database.InsertOrganization(r.Context(), database.InsertOrganizationParams{ - ID: uuid.New(), - Name: req.Name, - CreatedAt: database.Now(), - UpdatedAt: database.Now(), - }) - if err != nil { - return xerrors.Errorf("create organization: %w", err) - } - _, err = api.Database.InsertOrganizationMember(r.Context(), database.InsertOrganizationMemberParams{ - OrganizationID: organization.ID, - UserID: user.ID, - CreatedAt: database.Now(), - UpdatedAt: database.Now(), - Roles: []string{ - // Also assign member role incase they get demoted from admin - rbac.RoleOrgMember(organization.ID), - rbac.RoleOrgAdmin(organization.ID), - }, - }) - if err != nil { - return xerrors.Errorf("create organization member: %w", err) - } - return nil - }) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: err.Error(), - }) - return - } - - httpapi.Write(rw, http.StatusCreated, convertOrganization(organization)) -} - // Authenticates the user with an email and password. func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { var loginWithPassword codersdk.LoginWithPasswordRequest diff --git a/coderd/users_test.go b/coderd/users_test.go index b1c3bddc2e89c..d8d05542df092 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -174,7 +174,7 @@ func TestPostUsers(t *testing.T) { first := coderdtest.CreateFirstUser(t, client) notInOrg := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleAdmin(), rbac.RoleMember()) - org, err := other.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + org, err := other.CreateOrganization(context.Background(), codersdk.CreateOrganizationRequest{ Name: "another", }) require.NoError(t, err) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 1b841414cf531..b15a30f685c5a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -281,11 +281,18 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) // Create a new workspace for the currently authenticated user. func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Request) { + organization := httpmw.OrganizationParam(r) + apiKey := httpmw.APIKey(r) + if !api.Authorize(rw, r, rbac.ActionCreate, + rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(apiKey.UserID.String())) { + return + } + var createWorkspace codersdk.CreateWorkspaceRequest if !httpapi.Read(rw, r, &createWorkspace) { return } - apiKey := httpmw.APIKey(r) + template, err := api.Database.GetTemplateByID(r.Context(), createWorkspace.TemplateID) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ @@ -303,7 +310,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req }) return } - organization := httpmw.OrganizationParam(r) + if organization.ID != template.OrganizationID { httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ Message: fmt.Sprintf("template is not in organization %q", organization.Name), @@ -636,6 +643,9 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, workspace) { + return + } c, err := websocket.Accept(rw, r, &websocket.AcceptOptions{ // Fix for Safari 15.1: diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index aa57b528e77b2..cbf1c31c1dc96 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -81,7 +81,7 @@ func TestAdminViewAllWorkspaces(t *testing.T) { _, err := client.Workspace(context.Background(), workspace.ID) require.NoError(t, err) - otherOrg, err := client.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + otherOrg, err := client.CreateOrganization(context.Background(), codersdk.CreateOrganizationRequest{ Name: "default-test", }) require.NoError(t, err, "create other org") @@ -120,7 +120,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) { first := coderdtest.CreateFirstUser(t, client) other := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleMember(), rbac.RoleAdmin()) - org, err := other.CreateOrganization(context.Background(), codersdk.Me, codersdk.CreateOrganizationRequest{ + org, err := other.CreateOrganization(context.Background(), codersdk.CreateOrganizationRequest{ Name: "another", }) require.NoError(t, err) diff --git a/codersdk/users.go b/codersdk/users.go index 61faa398d0fa6..f594bf2b375b7 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -408,8 +408,8 @@ func (c *Client) OrganizationByName(ctx context.Context, user string, name strin } // CreateOrganization creates an organization and adds the provided user as an admin. -func (c *Client) CreateOrganization(ctx context.Context, user string, req CreateOrganizationRequest) (Organization, error) { - res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/organizations", user), req) +func (c *Client) CreateOrganization(ctx context.Context, req CreateOrganizationRequest) (Organization, error) { + res, err := c.Request(ctx, http.MethodPost, "/api/v2/organizations", req) if err != nil { return Organization{}, err } From 815e2e543e6006ea88ee4b6873c08499a8856233 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 27 May 2022 10:03:15 -0500 Subject: [PATCH 2/2] Fix post org route --- coderd/coderd.go | 54 ++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 85b790646aad8..f31474b879f67 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -138,37 +138,41 @@ func New(options *Options) *API { ) r.Get("/", api.provisionerDaemons) }) - r.Route("/organizations/{organization}", func(r chi.Router) { + r.Route("/organizations", func(r chi.Router) { r.Use( apiKeyMiddleware, - httpmw.ExtractOrganizationParam(options.Database), authRolesMiddleware, ) - r.Get("/", api.organization) r.Post("/", api.postOrganizations) - r.Post("/templateversions", api.postTemplateVersionsByOrganization) - r.Route("/templates", func(r chi.Router) { - r.Post("/", api.postTemplateByOrganization) - r.Get("/", api.templatesByOrganization) - r.Get("/{templatename}", api.templateByOrganizationAndName) - }) - r.Route("/workspaces", func(r chi.Router) { - r.Post("/", api.postWorkspacesByOrganization) - r.Get("/", api.workspacesByOrganization) - r.Route("/{user}", func(r chi.Router) { - r.Use(httpmw.ExtractUserParam(options.Database)) - r.Get("/{workspacename}", api.workspaceByOwnerAndName) - r.Get("/", api.workspacesByOwner) + r.Route("/{organization}", func(r chi.Router) { + r.Use( + httpmw.ExtractOrganizationParam(options.Database), + ) + r.Get("/", api.organization) + r.Post("/templateversions", api.postTemplateVersionsByOrganization) + r.Route("/templates", func(r chi.Router) { + r.Post("/", api.postTemplateByOrganization) + r.Get("/", api.templatesByOrganization) + r.Get("/{templatename}", api.templateByOrganizationAndName) }) - }) - r.Route("/members", func(r chi.Router) { - r.Get("/roles", api.assignableOrgRoles) - r.Route("/{user}", func(r chi.Router) { - r.Use( - httpmw.ExtractUserParam(options.Database), - httpmw.ExtractOrganizationMemberParam(options.Database), - ) - r.Put("/roles", api.putMemberRoles) + r.Route("/workspaces", func(r chi.Router) { + r.Post("/", api.postWorkspacesByOrganization) + r.Get("/", api.workspacesByOrganization) + r.Route("/{user}", func(r chi.Router) { + r.Use(httpmw.ExtractUserParam(options.Database)) + r.Get("/{workspacename}", api.workspaceByOwnerAndName) + r.Get("/", api.workspacesByOwner) + }) + }) + r.Route("/members", func(r chi.Router) { + r.Get("/roles", api.assignableOrgRoles) + r.Route("/{user}", func(r chi.Router) { + r.Use( + httpmw.ExtractUserParam(options.Database), + httpmw.ExtractOrganizationMemberParam(options.Database), + ) + r.Put("/roles", api.putMemberRoles) + }) }) }) })