Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Role permissions checks #170

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
111 changes: 111 additions & 0 deletions cmd/createrole.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package cmd

import (
"context"

"github.com/spf13/cobra"
"github.com/spf13/viper"
"go.infratographer.com/permissions-api/internal/config"
"go.infratographer.com/permissions-api/internal/iapl"
"go.infratographer.com/permissions-api/internal/query"
"go.infratographer.com/permissions-api/internal/spicedbx"
"go.infratographer.com/x/gidx"
"go.infratographer.com/x/viperx"
)

const (
createRoleFlagSubject = "subject"
createRoleFlagResource = "resource"
createRoleFlagActions = "actions"
)

var (
createRoleCmd = &cobra.Command{
Use: "create-role",
Short: "create role in SpiceDB directly",
Run: func(cmd *cobra.Command, args []string) {
createRole(cmd.Context(), globalCfg)
},
}
)

func init() {
rootCmd.AddCommand(createRoleCmd)

flags := createRoleCmd.Flags()
flags.String(createRoleFlagSubject, "", "subject to assign to created role")
flags.StringSlice(createRoleFlagActions, []string{}, "actions to assign to created role")
flags.String(createRoleFlagResource, "", "resource to bind to created role")

v := viper.GetViper()

viperx.MustBindFlag(v, createRoleFlagSubject, flags.Lookup(createRoleFlagSubject))
viperx.MustBindFlag(v, createRoleFlagActions, flags.Lookup(createRoleFlagActions))
viperx.MustBindFlag(v, createRoleFlagResource, flags.Lookup(createRoleFlagResource))
}

func createRole(ctx context.Context, cfg *config.AppConfig) {
subjectIDStr := viper.GetString(createRoleFlagSubject)
actions := viper.GetStringSlice(createRoleFlagActions)
resourceIDStr := viper.GetString(createRoleFlagResource)

if subjectIDStr == "" || len(actions) == 0 || resourceIDStr == "" {
logger.Fatal("invalid config")
}

spiceClient, err := spicedbx.NewClient(cfg.SpiceDB, cfg.Tracing.Enabled)
if err != nil {
logger.Fatalw("unable to initialize spicedb client", "error", err)
}

var policy iapl.Policy

if cfg.SpiceDB.PolicyFile != "" {
policy, err = iapl.NewPolicyFromFile(cfg.SpiceDB.PolicyFile)
if err != nil {
logger.Fatalw("unable to load new policy from schema file", "policy_file", cfg.SpiceDB.PolicyFile, "error", err)
}
} else {
logger.Warn("no spicedb policy file defined, using default policy")

policy = iapl.DefaultPolicy()
}

if err = policy.Validate(); err != nil {
logger.Fatalw("invalid spicedb policy", "error", err)
}

resourceID, err := gidx.Parse(resourceIDStr)
if err != nil {
logger.Fatalw("error parsing resource ID", "error", err)
}

subjectID, err := gidx.Parse(subjectIDStr)
if err != nil {
logger.Fatalw("error parsing subject ID", "error", err)
}

engine := query.NewEngine("infratographer", spiceClient, query.WithPolicy(policy), query.WithLogger(logger))

resource, err := engine.NewResourceFromID(resourceID)
if err != nil {
logger.Fatalw("error creating resource", "error", err)
}

subjectResource, err := engine.NewResourceFromID(subjectID)
if err != nil {
logger.Fatalw("error creating subject resource", "error", err)
}

role, _, err := engine.CreateRole(ctx, resource, actions)
if err != nil {
logger.Fatalw("error creating role", "error", err)
}

_, err = engine.AssignSubjectRole(ctx, subjectResource, role)
if err != nil {
logger.Fatalw("error creating role", "error", err)
}

logger.Infow("role successfully created", "role_id", role.ID)
}
58 changes: 35 additions & 23 deletions internal/api/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,37 @@ func (r *Router) checkAction(c echo.Context) error {
}

// Subject validation
subject, err := currentSubject(c)
subjectResource, err := r.currentSubject(c)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "failed to get the subject").SetInternal(err)
return err
}

subjectResource, err := r.engine.NewResourceFromID(subject)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error processing subject ID").SetInternal(err)
// Check the permissions
if err := r.checkActionWithResponse(ctx, subjectResource, action, resource); err != nil {
return err
}

// Check the permissions
err = r.engine.SubjectHasPermission(ctx, subjectResource, action, resource)
if err != nil && errors.Is(err, query.ErrActionNotAssigned) {
msg := fmt.Sprintf("subject '%s' does not have permission to perform action '%s' on resource '%s'",
subject, action, resourceIDStr)
return c.JSON(http.StatusOK, echo.Map{})
}

func (r *Router) checkActionWithResponse(ctx context.Context, subjectResource types.Resource, action string, resource types.Resource) error {
err := r.engine.SubjectHasPermission(ctx, subjectResource, action, resource)

switch {
case errors.Is(err, query.ErrActionNotAssigned):
msg := fmt.Sprintf(
"subject '%s' does not have permission to perform action '%s' on resource '%s'",
subjectResource.ID.String(),
action,
resource.ID.String(),
)

return echo.NewHTTPError(http.StatusForbidden, msg).SetInternal(err)
} else if err != nil {
case err != nil:
return echo.NewHTTPError(http.StatusInternalServerError, "an error occurred checking permissions").SetInternal(err)
default:
return nil
}

return c.JSON(http.StatusOK, echo.Map{})
}

type checkPermissionsRequest struct {
Expand Down Expand Up @@ -124,14 +133,9 @@ func (r *Router) checkAllActions(c echo.Context) error {
defer span.End()

// Subject validation
subject, err := currentSubject(c)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "failed to get the subject").SetInternal(err)
}

subjectResource, err := r.engine.NewResourceFromID(subject)
subjectResource, err := r.currentSubject(c)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error processing subject ID").SetInternal(err)
return err
}

var reqBody checkPermissionsRequest
Expand Down Expand Up @@ -223,8 +227,13 @@ func (r *Router) checkAllActions(c echo.Context) error {
case result := <-resultsCh:
if result.Error != nil {
if errors.Is(result.Error, query.ErrActionNotAssigned) {
err := fmt.Errorf("%w: subject '%s' does not have permission to perform action '%s' on resource '%s'",
ErrAccessDenied, subject, result.Request.Action, result.Request.Resource.ID.String())
err := fmt.Errorf(
"%w: subject '%s' does not have permission to perform action '%s' on resource '%s'",
ErrAccessDenied,
subjectResource.ID,
result.Request.Action,
result.Request.Resource.ID,
)

unauthorizedErrors++

Expand Down Expand Up @@ -254,7 +263,10 @@ func (r *Router) checkAllActions(c echo.Context) error {
}

if unauthorizedErrors != 0 {
msg := fmt.Sprintf("subject '%s' does not have permission to the requested resource actions", subject)
msg := fmt.Sprintf(
"subject '%s' does not have permission to the requested resource actions",
subjectResource.ID,
)

return echo.NewHTTPError(http.StatusForbidden, msg).SetInternal(multierr.Combine(allErrors...))
}
Expand Down
71 changes: 71 additions & 0 deletions internal/api/roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ import (
"go.opentelemetry.io/otel/trace"
)

const (
actionRoleCreate = "role_create"
actionRoleGet = "role_get"
actionRoleList = "role_list"
actionRoleUpdate = "role_update"
actionRoleDelete = "role_delete"
)

func (r *Router) roleCreate(c echo.Context) error {
resourceIDStr := c.Param("id")

Expand All @@ -27,11 +35,20 @@ func (r *Router) roleCreate(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error parsing request body").SetInternal(err)
}

subjectResource, err := r.currentSubject(c)
if err != nil {
return err
}

resource, err := r.engine.NewResourceFromID(resourceID)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error creating resource").SetInternal(err)
}

if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleCreate, resource); err != nil {
return err
}

role, _, err := r.engine.CreateRole(ctx, resource, reqBody.Actions)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error creating resource").SetInternal(err)
Expand All @@ -56,11 +73,29 @@ func (r *Router) roleGet(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

subjectResource, err := r.currentSubject(c)
if err != nil {
return err
}

roleResource, err := r.engine.NewResourceFromID(roleResourceID)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

// Roles belong to resources by way of the actions they can perform; do the permissions
// check on the role resource.
resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

// TODO: This shows an error for the role's resource, not the role. Determine if that
// matters.
if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleGet, resource); err != nil {
return err
}

role, err := r.engine.GetRole(ctx, roleResource, "")
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
Expand All @@ -85,11 +120,20 @@ func (r *Router) rolesList(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error parsing resource ID").SetInternal(err)
}

subjectResource, err := r.currentSubject(c)
if err != nil {
return err
}

resource, err := r.engine.NewResourceFromID(resourceID)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error creating resource").SetInternal(err)
}

if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleList, resource); err != nil {
return err
}

roles, err := r.engine.ListRoles(ctx, resource, "")
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err)
Expand Down Expand Up @@ -122,11 +166,27 @@ func (r *Router) roleDelete(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error deleting resource").SetInternal(err)
}

subjectResource, err := r.currentSubject(c)
if err != nil {
return err
}

roleResource, err := r.engine.NewResourceFromID(roleResourceID)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error deleting resource").SetInternal(err)
}

// Roles belong to resources by way of the actions they can perform; do the permissions
// check on the role resource.
resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleDelete, resource); err != nil {
return err
}

_, err = r.engine.DeleteRole(ctx, roleResource, "")
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error deleting resource").SetInternal(err)
Expand All @@ -150,16 +210,27 @@ func (r *Router) roleGetResource(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

subjectResource, err := r.currentSubject(c)
if err != nil {
return err
}

roleResource, err := r.engine.NewResourceFromID(roleResourceID)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

// There's a little irony here in that getting a role's resource here is required to actually
// do the permissions check.
resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}

if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleGet, resource); err != nil {
return err
}

resp := resourceResponse{
ID: resource.ID,
}
Expand Down
18 changes: 15 additions & 3 deletions internal/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package api

import (
"context"
"net/http"

"github.com/labstack/echo/v4"
"go.infratographer.com/permissions-api/internal/query"
"go.infratographer.com/permissions-api/internal/types"
"go.infratographer.com/x/echojwtx"
"go.infratographer.com/x/gidx"
"go.opentelemetry.io/otel"
Expand Down Expand Up @@ -97,8 +99,18 @@ func WithCheckConcurrency(count int) Option {
}
}

func currentSubject(c echo.Context) (gidx.PrefixedID, error) {
subject := echojwtx.Actor(c)
func (r *Router) currentSubject(c echo.Context) (types.Resource, error) {
subjectStr := echojwtx.Actor(c)

return gidx.Parse(subject)
subject, err := gidx.Parse(subjectStr)
if err != nil {
return types.Resource{}, echo.NewHTTPError(http.StatusBadRequest, "failed to get the subject").SetInternal(err)
}

subjectResource, err := r.engine.NewResourceFromID(subject)
if err != nil {
return types.Resource{}, echo.NewHTTPError(http.StatusBadRequest, "error processing subject ID").SetInternal(err)
}

return subjectResource, nil
}
3 changes: 2 additions & 1 deletion permissions-api.example.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
oidc:
issuer: http://mock-oauth2-server:8081/default

spicedb:
policyFile: /workspace/policy.example.yaml
Loading
Loading