Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: Add rbac to templateversion+orgmember endpoints#1713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 10 commits intomainfromstevenmasley/rbac_template_versions
May 25, 2022
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletionscoderd/coderd.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -170,6 +170,7 @@ func newRouter(options *Options, a *api) chi.Router {
r.Route("/{user}", func(r chi.Router) {
r.Use(
httpmw.ExtractUserParam(options.Database),
httpmw.ExtractOrganizationMemberParam(options.Database),
)
r.Put("/roles", a.putMemberRoles)
})
Expand DownExpand Up@@ -201,6 +202,7 @@ func newRouter(options *Options, a *api) chi.Router {
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
r.Use(
apiKeyMiddleware,
authRolesMiddleware,
httpmw.ExtractTemplateVersionParam(options.Database),
)

Expand DownExpand Up@@ -289,6 +291,7 @@ func newRouter(options *Options, a *api) chi.Router {
r.Route("/workspaceresources/{workspaceresource}", func(r chi.Router) {
r.Use(
apiKeyMiddleware,
authRolesMiddleware,
httpmw.ExtractWorkspaceResourceParam(options.Database),
httpmw.ExtractWorkspaceParam(options.Database),
)
Expand Down
95 changes: 74 additions & 21 deletionscoderd/coderd_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,8 @@ import (
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/provisioner/echo"
"github.com/coder/coder/provisionersdk/proto"
)

func TestMain(m *testing.M) {
Expand DownExpand Up@@ -47,13 +49,32 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
require.NoError(t, err, "fetch org")

// Setup some data in the database.
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
Provision: []*proto.Provision_Response{{
Type: &proto.Provision_Response_Complete{
Complete: &proto.Provision_Complete{
// Return a workspace resource
Resources: []*proto.Resource{{
Name: "some",
Type: "example",
Agents: []*proto.Agent{{
Id: "something",
Auth: &proto.Agent_Token{},
}},
}},
},
},
}},
})
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, admin.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
file, err := client.Upload(ctx, codersdk.ContentTypeTar, make([]byte, 1024))
require.NoError(t, err, "upload file")
workspaceResources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
require.NoError(t, err, "workspace resources")

// Always fail auth from this point forward
authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
Expand All@@ -78,6 +99,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
"POST:/api/v2/users/logout": {NoAuthorize: true},
"GET:/api/v2/users/authmethods": {NoAuthorize: true},

// Has it's own auth
"GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true},

// All workspaceagents endpoints do not use rbac
"POST:/api/v2/workspaceagents/aws-instance-identity": {NoAuthorize: true},
"POST:/api/v2/workspaceagents/azure-instance-identity": {NoAuthorize: true},
Expand All@@ -94,11 +118,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
"GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true},

// TODO: @emyrk these need to be fixed by adding authorize calls
"GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize: true},

"GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true},

"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true},
"GET:/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize: true},
"GET:/api/v2/organizations/{organization}/templates/{templatename}": {NoAuthorize: true},
"POST:/api/v2/organizations/{organization}/templateversions": {NoAuthorize: true},
Expand All@@ -108,17 +127,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
"GET:/api/v2/parameters/{scope}/{id}": {NoAuthorize: true},
"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {NoAuthorize: true},

"GET:/api/v2/templates/{template}/versions": {NoAuthorize: true},
"PATCH:/api/v2/templates/{template}/versions": {NoAuthorize: true},
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {NoAuthorize: true},

"GET:/api/v2/templateversions/{templateversion}": {NoAuthorize: true},
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {NoAuthorize: true},
"GET:/api/v2/templateversions/{templateversion}/logs": {NoAuthorize: true},
"GET:/api/v2/templateversions/{templateversion}/parameters": {NoAuthorize: true},
"GET:/api/v2/templateversions/{templateversion}/resources": {NoAuthorize: true},
"GET:/api/v2/templateversions/{templateversion}/schema": {NoAuthorize: true},

"POST:/api/v2/users/{user}/organizations": {NoAuthorize: true},

"GET:/api/v2/workspaces/{workspace}/watch": {NoAuthorize: true},
Expand DownExpand Up@@ -164,6 +172,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
AssertAction: rbac.ActionUpdate,
AssertObject: workspaceRBACObj,
},
"GET:/api/v2/workspaceresources/{workspaceresource}": {
AssertAction: rbac.ActionRead,
AssertObject: workspaceRBACObj,
},
"PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": {
AssertAction: rbac.ActionUpdate,
AssertObject: workspaceRBACObj,
Expand DownExpand Up@@ -199,12 +211,51 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
"GET:/api/v2/files/{fileHash}": {AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash)},
"GET:/api/v2/files/{fileHash}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash),
},
"GET:/api/v2/templates/{template}/versions": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"PATCH:/api/v2/templates/{template}/versions": {
AssertAction: rbac.ActionUpdate,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"GET:/api/v2/templateversions/{templateversion}": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {
AssertAction: rbac.ActionUpdate,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"GET:/api/v2/templateversions/{templateversion}/logs": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"GET:/api/v2/templateversions/{templateversion}/parameters": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"GET:/api/v2/templateversions/{templateversion}/resources": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},
"GET:/api/v2/templateversions/{templateversion}/schema": {
AssertAction: rbac.ActionRead,
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
},

// 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},
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"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},
}

for k, v := range assertRoute {
Expand DownExpand Up@@ -240,6 +291,8 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name)
route = strings.ReplaceAll(route, "{template}", template.ID.String())
route = strings.ReplaceAll(route, "{hash}", file.Hash)
route = strings.ReplaceAll(route, "{workspaceresource}", workspaceResources[0].ID.String())
route = strings.ReplaceAll(route, "{templateversion}", version.ID.String())

resp, err := client.Request(context.Background(), method, route, nil)
require.NoError(t, err, "do req")
Expand Down
3 changes: 1 addition & 2 deletionscoderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -276,8 +276,7 @@ func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uui
for orgID, roles := range orgRoles {
organizationID, err := uuid.Parse(orgID)
require.NoError(t, err, fmt.Sprintf("parse org id %q", orgID))
// TODO: @Emyrk add the member to the organization if they do not already belong.
_, err = other.UpdateOrganizationMemberRoles(context.Background(), organizationID, user.ID.String(),
_, err = client.UpdateOrganizationMemberRoles(context.Background(), organizationID, user.ID.String(),
codersdk.UpdateRoles{Roles: append(roles, rbac.RoleOrgMember(organizationID))})
require.NoError(t, err, "update org membership roles")
}
Expand Down
5 changes: 5 additions & 0 deletionscoderd/database/modelmethods.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,11 @@ func (t Template) RBACObject() rbac.Object {
return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.ID.String())
}

func (t TemplateVersion) RBACObject() rbac.Object {
// Just use the parent template resource for controlling versions
return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.TemplateID.UUID.String())
}

func (w Workspace) RBACObject() rbac.Object {
return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithID(w.ID.String()).WithOwner(w.OwnerID.String())
}
Expand Down
23 changes: 17 additions & 6 deletionscoderd/httpmw/organizationparam.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,12 +28,12 @@ func OrganizationParam(r *http.Request) database.Organization {
funcOrganizationMemberParam(r*http.Request) database.OrganizationMember {
organizationMember,ok:=r.Context().Value(organizationMemberParamContextKey{}).(database.OrganizationMember)
if!ok {
panic("developer error: organization param middleware not provided")
panic("developer error: organizationmemberparam middleware not provided")
}
returnorganizationMember
}

// ExtractOrganizationParam grabs an organizationand user membershipfrom the "organization" URL parameter.
// ExtractOrganizationParam grabs an organization from the "organization" URL parameter.
// This middleware requires the API key middleware higher in the call stack for authentication.
funcExtractOrganizationParam(db database.Store)func(http.Handler) http.Handler {
returnfunc(next http.Handler) http.Handler {
Expand All@@ -56,11 +56,23 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
})
return
}
ctx:=context.WithValue(r.Context(),organizationParamContextKey{},organization)
next.ServeHTTP(rw,r.WithContext(ctx))
})
}
}

// ExtractOrganizationMemberParam grabs a user membership from the "organization" and "user" URL parameter.
// This middleware requires the ExtractUser and ExtractOrganization middleware higher in the stack
funcExtractOrganizationMemberParam(db database.Store)func(http.Handler) http.Handler {
Comment on lines +65 to +67
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

BeforeExtractOrganization did this with theAPIKey.UserID. That didn't make sense to me, as we use{user} forExtractUser.

So I changed this to use the user from{user} and org from{organization}

returnfunc(next http.Handler) http.Handler {
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
organization:=OrganizationParam(r)
user:=UserParam(r)

apiKey:=APIKey(r)
organizationMember,err:=db.GetOrganizationMemberByUserID(r.Context(), database.GetOrganizationMemberByUserIDParams{
OrganizationID:organization.ID,
UserID:apiKey.UserID,
UserID:user.ID,
})
iferrors.Is(err,sql.ErrNoRows) {
httpapi.Write(rw,http.StatusForbidden, httpapi.Response{
Expand All@@ -74,9 +86,8 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
})
return
}
ctx:=context.WithValue(r.Context(),organizationMemberParamContextKey{},organizationMember)

ctx:=context.WithValue(r.Context(),organizationParamContextKey{},organization)
ctx=context.WithValue(ctx,organizationMemberParamContextKey{},organizationMember)
next.ServeHTTP(rw,r.WithContext(ctx))
})
}
Expand Down
8 changes: 7 additions & 1 deletioncoderd/httpmw/organizationparam_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ func TestOrganizationParam(t *testing.T) {
var (
db = databasefake.New()
rw = httptest.NewRecorder()
r,_ = setupAuthentication(db)
r,u = setupAuthentication(db)
rtr = chi.NewRouter()
)
organization, err := db.InsertOrganization(r.Context(), database.InsertOrganizationParams{
Expand All@@ -133,9 +133,12 @@ func TestOrganizationParam(t *testing.T) {
})
require.NoError(t, err)
chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String())
chi.RouteContext(r.Context()).URLParams.Add("user", u.ID.String())
rtr.Use(
httpmw.ExtractAPIKey(db, nil),
httpmw.ExtractUserParam(db),
httpmw.ExtractOrganizationParam(db),
httpmw.ExtractOrganizationMemberParam(db),
)
rtr.Get("/", nil)
rtr.ServeHTTP(rw, r)
Expand DownExpand Up@@ -167,9 +170,12 @@ func TestOrganizationParam(t *testing.T) {
})
require.NoError(t, err)
chi.RouteContext(r.Context()).URLParams.Add("organization", organization.ID.String())
chi.RouteContext(r.Context()).URLParams.Add("user", user.ID.String())
rtr.Use(
httpmw.ExtractAPIKey(db, nil),
httpmw.ExtractOrganizationParam(db),
httpmw.ExtractUserParam(db),
httpmw.ExtractOrganizationMemberParam(db),
)
rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) {
_ = httpmw.OrganizationParam(r)
Expand Down
28 changes: 15 additions & 13 deletionscoderd/members.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,27 +17,29 @@ import (
)

func (api *api) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
// User is the user to modify
// TODO: Until rbac authorize is implemented, only be able to change your
//own roles. This also means you can grant yourself whatever roles you want.
user := httpmw.UserParam(r)
apiKey := httpmw.APIKey(r)
organization := httpmw.OrganizationParam(r)
// TODO: @emyrk add proper `Authorize()` check here instead of a uuid match.
//Proper authorize should check the granted roles are able to given within
//the selected organization. Until then, allow anarchy
if apiKey.UserID != user.ID {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: "modifying other users is not supported at this time",
})
return
}
member := httpmw.OrganizationMemberParam(r)

var params codersdk.UpdateRoles
if !httpapi.Read(rw, r, &params) {
return
}

added, removed := rbac.ChangeRoleSet(member.Roles, params.Roles)
for _, roleName := range added {
// Assigning a role requires the create permission.
if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
return
}
}
for _, roleName := range removed {
// Removing a role requires the delete permission.
if !api.Authorize(rw, r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
return
}
}

updatedUser, err := api.updateOrganizationMemberRoles(r.Context(), database.UpdateMemberRolesParams{
GrantedRoles: params.Roles,
UserID: user.ID,
Expand Down
1 change: 1 addition & 0 deletionscoderd/rbac/authz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,7 @@ package rbac
import (
"context"
_ "embed"

"golang.org/x/xerrors"

"github.com/open-policy-agent/opa/rego"
Expand Down
37 changes: 37 additions & 0 deletionscoderd/rbac/builtin.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,12 @@ var (
Action: ActionRead,
ResourceID: "*",
},
{
// Can read available roles.
ResourceType: ResourceOrgRoleAssignment.Type,
ResourceID: "*",
Action: ActionRead,
},
},
},
}
Expand DownExpand Up@@ -217,6 +223,37 @@ func SiteRoles() []Role {
return roles
}

// ChangeRoleSet is a helper function that finds the difference of 2 sets of
// roles. When setting a user's new roles, it is equivalent to adding and
// removing roles. This set determines the changes, so that the appropriate
// RBAC checks can be applied using "ActionCreate" and "ActionDelete" for
// "added" and "removed" roles respectively.
func ChangeRoleSet(from []string, to []string) (added []string, removed []string) {
has := make(map[string]struct{})
for _, exists := range from {
has[exists] = struct{}{}
}

for _, roleName := range to {
// If the user already has the role assigned, we don't need to check the permission
// to reassign it. Only run permission checks on the difference in the set of
// roles.
if _, ok := has[roleName]; ok {
delete(has, roleName)
continue
}

added = append(added, roleName)
}

// Remaining roles are the ones removed/deleted.
for roleName := range has {
removed = append(removed, roleName)
}

return added, removed
}

// roleName is a quick helper function to return
// role_name:scopeID
// If no scopeID is required, only 'role_name' is returned
Expand Down
Loading

[8]ページ先頭

©2009-2025 Movatter.jp