Skip to content

Commit

Permalink
add manager field to simplify multi service management (#292)
Browse files Browse the repository at this point in the history 
This adds a manager field to roles and role bindings allowing services
to mark the manager of a specific role / role binding.

This allows the managing service to easily filter out unmanaged roles / role bindings
which it does not own which allows for cleaner logic for reconciling roles/role bindings.

Both roles and role binding create requests can provide an optional `manager` field
and role / role binding list endpoints can now be provided an optional `manager` query value.

Signed-off-by: Mike Mason <[email protected]>
  • Loading branch information
@mikemrm
mikemrm authored Oct 2, 2024
1 parent 73e75eb commit 10238a4
Show file tree
Hide file tree
Showing 21 changed files with 509 additions and 115 deletions.
4 changes: 2 additions & 2 deletions cmd/createrole.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func createRole(ctx context.Context, cfg *config.AppConfig) {
logger.Fatalw("error creating subject resource", "error", err)
}

role, err := engine.CreateRoleV2(ctx, subjectResource, resource, name, actions)
role, err := engine.CreateRoleV2(ctx, subjectResource, resource, "", name, actions)
if err != nil {
logger.Fatalw("error creating role", "error", err)
}
Expand All @@ -125,7 +125,7 @@ func createRole(ctx context.Context, cfg *config.AppConfig) {
logger.Fatalw("error creating role resource", "error", err)
}

rb, err := engine.CreateRoleBinding(ctx, subjectResource, resource, roleres, rbsubj)
rb, err := engine.CreateRoleBinding(ctx, subjectResource, resource, roleres, "", rbsubj)
if err != nil {
logger.Fatalw("error creating role binding", "error", err)
}
Expand Down
17 changes: 15 additions & 2 deletions internal/api/rolebindings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (r *Router) roleBindingCreate(c echo.Context) error {
}
}

rb, err := r.engine.CreateRoleBinding(ctx, actor, resource, roleResource, subjects)
rb, err := r.engine.CreateRoleBinding(ctx, actor, resource, roleResource, body.Manager, subjects)
if err != nil {
return r.errorResponse("error creating role-binding", err)
}
Expand All @@ -83,6 +83,7 @@ func (r *Router) roleBindingCreate(c echo.Context) error {
roleBindingResponse{
ID: rb.ID,
ResourceID: rb.ResourceID,
Manager: rb.Manager,
SubjectIDs: rb.SubjectIDs,
RoleID: rb.RoleID,

Expand Down Expand Up @@ -122,7 +123,16 @@ func (r *Router) roleBindingsList(c echo.Context) error {
return err
}

rbs, err := r.engine.ListRoleBindings(ctx, resource, nil)
var rbs []types.RoleBinding

params := c.QueryParams()

if params.Has("manager") {
rbs, err = r.engine.ListManagerRoleBindings(ctx, params.Get("manager"), resource, nil)
} else {
rbs, err = r.engine.ListRoleBindings(ctx, resource, nil)
}

if err != nil {
return r.errorResponse("error listing role-binding", err)
}
Expand All @@ -137,6 +147,7 @@ func (r *Router) roleBindingsList(c echo.Context) error {
ResourceID: rb.ResourceID,
SubjectIDs: rb.SubjectIDs,
RoleID: rb.RoleID,
Manager: rb.Manager,

CreatedBy: rb.CreatedBy,
UpdatedBy: rb.UpdatedBy,
Expand Down Expand Up @@ -242,6 +253,7 @@ func (r *Router) roleBindingGet(c echo.Context) error {
ResourceID: rb.ResourceID,
SubjectIDs: rb.SubjectIDs,
RoleID: rb.RoleID,
Manager: rb.Manager,

CreatedBy: rb.CreatedBy,
UpdatedBy: rb.UpdatedBy,
Expand Down Expand Up @@ -321,6 +333,7 @@ func (r *Router) roleBindingUpdate(c echo.Context) error {
ResourceID: rb.ResourceID,
SubjectIDs: rb.SubjectIDs,
RoleID: rb.RoleID,
Manager: rb.Manager,

CreatedBy: rb.CreatedBy,
UpdatedBy: rb.UpdatedBy,
Expand Down
18 changes: 16 additions & 2 deletions internal/api/roles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"go.infratographer.com/permissions-api/internal/iapl"
"go.infratographer.com/permissions-api/internal/query"
"go.infratographer.com/permissions-api/internal/storage"
"go.infratographer.com/permissions-api/internal/types"
)

func (r *Router) roleCreate(c echo.Context) error {
Expand Down Expand Up @@ -49,7 +50,7 @@ func (r *Router) roleCreate(c echo.Context) error {
}

role, err := r.engine.CreateRole(
ctx, subjectResource, resource,
ctx, subjectResource, resource, reqBody.Manager,
strings.TrimSpace(reqBody.Name), reqBody.Actions,
)

Expand All @@ -66,6 +67,7 @@ func (r *Router) roleCreate(c echo.Context) error {
resp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
ResourceID: role.ResourceID,
CreatedBy: role.CreatedBy,
Expand Down Expand Up @@ -139,6 +141,7 @@ func (r *Router) roleUpdate(c echo.Context) error {
resp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
ResourceID: role.ResourceID,
CreatedBy: role.CreatedBy,
Expand Down Expand Up @@ -202,6 +205,7 @@ func (r *Router) roleGet(c echo.Context) error {
resp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
ResourceID: role.ResourceID,
CreatedBy: role.CreatedBy,
Expand Down Expand Up @@ -238,7 +242,16 @@ func (r *Router) rolesList(c echo.Context) error {
return err
}

roles, err := r.engine.ListRoles(ctx, resource)
var roles []types.Role

params := c.QueryParams()

if params.Has("manager") {
roles, err = r.engine.ListManagerRoles(ctx, params.Get("manager"), resource)
} else {
roles, err = r.engine.ListRoles(ctx, resource)
}

if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err)
}
Expand All @@ -251,6 +264,7 @@ func (r *Router) rolesList(c echo.Context) error {
roleResp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
CreatedBy: role.CreatedBy,
UpdatedBy: role.UpdatedBy,
Expand Down
22 changes: 18 additions & 4 deletions internal/api/roles_v2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"go.infratographer.com/permissions-api/internal/iapl"
"go.infratographer.com/permissions-api/internal/types"

"github.com/labstack/echo/v4"
"go.infratographer.com/x/gidx"
Expand Down Expand Up @@ -47,7 +48,7 @@ func (r *Router) roleV2Create(c echo.Context) error {
}

role, err := r.engine.CreateRoleV2(
ctx, subjectResource, resource,
ctx, subjectResource, resource, reqBody.Manager,
strings.TrimSpace(reqBody.Name), reqBody.Actions,
)
if err != nil {
Expand All @@ -57,6 +58,7 @@ func (r *Router) roleV2Create(c echo.Context) error {
resp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
ResourceID: role.ResourceID,
CreatedBy: role.CreatedBy,
Expand Down Expand Up @@ -113,6 +115,7 @@ func (r *Router) roleV2Update(c echo.Context) error {
resp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
ResourceID: role.ResourceID,
CreatedBy: role.CreatedBy,
Expand Down Expand Up @@ -159,6 +162,7 @@ func (r *Router) roleV2Get(c echo.Context) error {
resp := roleResponse{
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
Actions: role.Actions,
ResourceID: role.ResourceID,
CreatedBy: role.CreatedBy,
Expand Down Expand Up @@ -195,7 +199,16 @@ func (r *Router) roleV2sList(c echo.Context) error {
return err
}

roles, err := r.engine.ListRolesV2(ctx, resource)
var roles []types.Role

params := c.QueryParams()

if params.Has("manager") {
roles, err = r.engine.ListManagerRolesV2(ctx, params.Get("manager"), resource)
} else {
roles, err = r.engine.ListRolesV2(ctx, resource)
}

if err != nil {
return r.errorResponse("error getting roles", err)
}
Expand All @@ -206,8 +219,9 @@ func (r *Router) roleV2sList(c echo.Context) error {

for _, role := range roles {
roleResp := listRolesV2Role{
ID: role.ID,
Name: role.Name,
ID: role.ID,
Name: role.Name,
Manager: role.Manager,
}

resp.Data = append(resp.Data, roleResp)
Expand Down
9 changes: 7 additions & 2 deletions internal/api/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

type createRoleRequest struct {
Name string `json:"name" binding:"required"`
Manager string `json:"manager"`
Actions []string `json:"actions" binding:"required"`
}

Expand All @@ -17,6 +18,7 @@ type updateRoleRequest struct {
type roleResponse struct {
ID gidx.PrefixedID `json:"id"`
Name string `json:"name"`
Manager string `json:"manager,omitempty"`
Actions []string `json:"actions"`

ResourceID gidx.PrefixedID `json:"resource_id,omitempty"`
Expand Down Expand Up @@ -77,15 +79,17 @@ type listRolesV2Response struct {
}

type listRolesV2Role struct {
ID gidx.PrefixedID `json:"id"`
Name string `json:"name"`
ID gidx.PrefixedID `json:"id"`
Name string `json:"name"`
Manager string `json:"manager"`
}

// RoleBindings

type roleBindingRequest struct {
RoleID string `json:"role_id" binding:"required"`
SubjectIDs []gidx.PrefixedID `json:"subject_ids" binding:"required"`
Manager string `json:"manager"`
}

type rolebindingUpdateRequest struct {
Expand All @@ -96,6 +100,7 @@ type roleBindingResponse struct {
ID gidx.PrefixedID `json:"id"`
ResourceID gidx.PrefixedID `json:"resource_id"`
RoleID gidx.PrefixedID `json:"role_id"`
Manager string `json:"manager"`
SubjectIDs []gidx.PrefixedID `json:"subject_ids"`

CreatedBy gidx.PrefixedID `json:"created_by"`
Expand Down
14 changes: 7 additions & 7 deletions internal/query/example_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,21 +179,21 @@ func TestExamplePolicy(t *testing.T) {
require.NoError(t, err)

// create roles
superadmin, err := e.CreateRoleV2(ctx, superuser, tnnttenroot, "superuser", allactions)
superadmin, err := e.CreateRoleV2(ctx, superuser, tnnttenroot, t.Name(), "superuser", allactions)
require.NoError(t, err)

iamadmin, err := e.CreateRoleV2(ctx, superuser, tnnttena, "iam_admin", iamactions)
iamadmin, err := e.CreateRoleV2(ctx, superuser, tnnttena, t.Name(), "iam_admin", iamactions)
require.NoError(t, err)

lbadmin, err := e.CreateRoleV2(ctx, superuser, tnnttenroot, "lb_admin", lbactions)
lbadmin, err := e.CreateRoleV2(ctx, superuser, tnnttenroot, t.Name(), "lb_admin", lbactions)
require.NoError(t, err)

tc := []testingx.TestCase[any, any]{
{
Name: "superuser can do anything",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
role := types.Resource{Type: "role", ID: superadmin.ID}
_, err := e.CreateRoleBinding(ctx, superuser, tnnttenroot, role, []types.RoleBindingSubject{{SubjectResource: superuser}})
_, err := e.CreateRoleBinding(ctx, superuser, tnnttenroot, role, t.Name(), []types.RoleBindingSubject{{SubjectResource: superuser}})
require.NoError(t, err)

return ctx
Expand Down Expand Up @@ -229,7 +229,7 @@ func TestExamplePolicy(t *testing.T) {
Sync: true,
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
role := types.Resource{Type: "role", ID: lbadmin.ID}
_, err := e.CreateRoleBinding(ctx, superuser, tnnttena, role, []types.RoleBindingSubject{{SubjectResource: groupadmin}})
_, err := e.CreateRoleBinding(ctx, superuser, tnnttena, role, t.Name(), []types.RoleBindingSubject{{SubjectResource: groupadmin}})
require.NoError(t, err)

return ctx
Expand Down Expand Up @@ -279,7 +279,7 @@ func TestExamplePolicy(t *testing.T) {
Sync: true,
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
role := types.Resource{Type: "role", ID: iamadmin.ID}
_, err := e.CreateRoleBinding(ctx, superuser, tnnttena, role, []types.RoleBindingSubject{{SubjectResource: groupadminsubgroup}})
_, err := e.CreateRoleBinding(ctx, superuser, tnnttena, role, t.Name(), []types.RoleBindingSubject{{SubjectResource: groupadminsubgroup}})
require.NoError(t, err)

return ctx
Expand Down Expand Up @@ -326,7 +326,7 @@ func TestExamplePolicy(t *testing.T) {
Name: "iam-admin cannot be bind on tnntten-root",
CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[any]) {
role := types.Resource{Type: "role", ID: iamadmin.ID}
_, err := e.CreateRoleBinding(ctx, superuser, tnnttenroot, role, []types.RoleBindingSubject{{SubjectResource: groupadminsubgroup}})
_, err := e.CreateRoleBinding(ctx, superuser, tnnttenroot, role, t.Name(), []types.RoleBindingSubject{{SubjectResource: groupadminsubgroup}})
assert.Error(t, err)
assert.ErrorIs(t, err, ErrRoleNotFound)
},
Expand Down
21 changes: 18 additions & 3 deletions internal/query/mock/mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (e *Engine) CreateRelationships(context.Context, []types.Relationship) erro
}

// CreateRole creates a Role object and does not persist it anywhere.
func (e *Engine) CreateRole(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) {
func (e *Engine) CreateRole(context.Context, types.Resource, types.Resource, string, string, []string) (types.Role, error) {
args := e.Called()

retRole := args.Get(0).(types.Role)
Expand All @@ -60,7 +60,7 @@ func (e *Engine) CreateRole(context.Context, types.Resource, types.Resource, str

// CreateRoleV2 creates a v2 role object
// TODO: Implement this
func (e *Engine) CreateRoleV2(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) {
func (e *Engine) CreateRoleV2(context.Context, types.Resource, types.Resource, string, string, []string) (types.Role, error) {
return types.Role{}, nil
}

Expand All @@ -69,6 +69,11 @@ func (e *Engine) ListRolesV2(context.Context, types.Resource) ([]types.Role, err
return nil, nil
}

// ListManagerRolesV2 list roles
func (e *Engine) ListManagerRolesV2(context.Context, string, types.Resource) ([]types.Role, error) {
return nil, nil
}

// UpdateRole returns the provided mock results.
func (e *Engine) UpdateRole(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) {
args := e.Called()
Expand Down Expand Up @@ -130,6 +135,11 @@ func (e *Engine) ListRoles(context.Context, types.Resource) ([]types.Role, error
return nil, nil
}

// ListManagerRoles returns nothing but satisfies the Engine interface.
func (e *Engine) ListManagerRoles(context.Context, string, types.Resource) ([]types.Role, error) {
return nil, nil
}

// DeleteRelationships does nothing but satisfies the Engine interface.
func (e *Engine) DeleteRelationships(context.Context, ...types.Relationship) error {
args := e.Called()
Expand Down Expand Up @@ -209,7 +219,7 @@ func (e *Engine) SubjectHasPermission(context.Context, types.Resource, string, t
}

// CreateRoleBinding returns nothing but satisfies the Engine interface.
func (e *Engine) CreateRoleBinding(context.Context, types.Resource, types.Resource, types.Resource, []types.RoleBindingSubject) (types.RoleBinding, error) {
func (e *Engine) CreateRoleBinding(context.Context, types.Resource, types.Resource, types.Resource, string, []types.RoleBindingSubject) (types.RoleBinding, error) {
return types.RoleBinding{}, nil
}

Expand All @@ -218,6 +228,11 @@ func (e *Engine) ListRoleBindings(context.Context, types.Resource, *types.Resour
return nil, nil
}

// ListManagerRoleBindings returns nothing but satisfies the Engine interface.
func (e *Engine) ListManagerRoleBindings(context.Context, string, types.Resource, *types.Resource) ([]types.RoleBinding, error) {
return nil, nil
}

// GetRoleBinding returns nothing but satisfies the Engine interface.
func (e *Engine) GetRoleBinding(context.Context, types.Resource) (types.RoleBinding, error) {
return types.RoleBinding{}, nil
Expand Down
Loading

0 comments on commit 10238a4

Please sign in to comment.