Skip to content

Commit

Permalink
update authzed-go with PermissionService reqs /w context
Browse files Browse the repository at this point in the history
- fully implemented in Check API, new permissionship value
  "conditional" returned if fields were missing in context
- LookupResources and LookupSubjects return errors when a
  caveated relationship is detected in the working set
- a limit of 4096 bytes in the caveat context was
  introduced. Eventually we can turn this into a configurable
  setting
  • Loading branch information
vroldanbet committed Oct 18, 2022
1 parent 89d488a commit 97069de
Show file tree
Hide file tree
Showing 13 changed files with 281 additions and 20 deletions.
2 changes: 1 addition & 1 deletion e2e/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/authzed/spicedb/e2e
go 1.18

require (
github.com/authzed/authzed-go v0.7.1-0.20220930215614-6cee7a807da2
github.com/authzed/authzed-go v0.7.1-0.20221014175106-c7eaea237a32
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5
github.com/authzed/spicedb v1.5.0
github.com/brianvoe/gofakeit/v6 v6.15.0
Expand Down
4 changes: 2 additions & 2 deletions e2e/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/authzed/authzed-go v0.7.1-0.20220930215614-6cee7a807da2 h1:Uo5FOj/e3mPqnRkvWV8zg4nFhMFHXPKq1lNZIeHxvgg=
github.com/authzed/authzed-go v0.7.1-0.20220930215614-6cee7a807da2/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/authzed-go v0.7.1-0.20221014175106-c7eaea237a32 h1:xaiUxYuPpyNMiATRCfUJinEC8vXKqJYD9RZwtASAesw=
github.com/authzed/authzed-go v0.7.1-0.20221014175106-c7eaea237a32/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5 h1:sZM7XzdyuLyxj7pC/g7uX+XAqZ7m6NMxZzuQRovgBPw=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5/go.mod h1:rqjY3zyK/YP7NID9+B2BdIRRkvnK+cdf9/qya/zaFZE=
github.com/brianvoe/gofakeit/v6 v6.15.0 h1:lJPGJZ2/07TRGDazyTzD5b18N3y4tmmJpdhCUw18FlI=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
cloud.google.com/go/spanner v1.39.0
github.com/IBM/pgxpoolprometheus v1.0.1
github.com/Masterminds/squirrel v1.5.3
github.com/authzed/authzed-go v0.7.1-0.20220930215614-6cee7a807da2
github.com/authzed/authzed-go v0.7.1-0.20221014175106-c7eaea237a32
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5
github.com/aws/aws-sdk-go v1.44.110
github.com/benbjohnson/clock v1.3.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk5
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/antlr/antlr4/runtime/Go/antlr v0.0.0-20220418222510-f25a4f6275ed h1:ue9pVfIcP+QMEjfgo/Ez4ZjNZfonGgR6NgjMaJMu1Cg=
github.com/antlr/antlr4/runtime/Go/antlr v0.0.0-20220418222510-f25a4f6275ed/go.mod h1:F7bn7fEU90QkQ3tnmaTx3LTKLEDqnwWODIYppRQ5hnY=
github.com/authzed/authzed-go v0.7.1-0.20220930215614-6cee7a807da2 h1:Uo5FOj/e3mPqnRkvWV8zg4nFhMFHXPKq1lNZIeHxvgg=
github.com/authzed/authzed-go v0.7.1-0.20220930215614-6cee7a807da2/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/authzed-go v0.7.1-0.20221014175106-c7eaea237a32 h1:xaiUxYuPpyNMiATRCfUJinEC8vXKqJYD9RZwtASAesw=
github.com/authzed/authzed-go v0.7.1-0.20221014175106-c7eaea237a32/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5 h1:sZM7XzdyuLyxj7pC/g7uX+XAqZ7m6NMxZzuQRovgBPw=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5/go.mod h1:rqjY3zyK/YP7NID9+B2BdIRRkvnK+cdf9/qya/zaFZE=
github.com/aws/aws-sdk-go v1.17.4/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
Expand Down
12 changes: 12 additions & 0 deletions internal/graph/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,15 @@ func NewErrInvalidArgument(baseErr error) error {
error: baseErr,
}
}

// ErrUnimplemented is returned when some functionality is not yet supported.
type ErrUnimplemented struct {
error
}

// NewUnimplementedErr constructs a new unimplemented error.
func NewUnimplementedErr(baseErr error) error {
return ErrUnimplemented{
error: baseErr,
}
}
3 changes: 1 addition & 2 deletions internal/graph/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package graph
import (
"context"
"errors"
"fmt"
"sync"

"github.com/shopspring/decimal"
Expand Down Expand Up @@ -105,7 +104,7 @@ func (cl *ConcurrentLookup) LookupViaReachability(ctx context.Context, req Valid
Metadata: req.Metadata,
}, stream)
if err != nil {
resp := lookupResultError(NewErrInvalidArgument(fmt.Errorf("error in reachablility: %w", err)), emptyMetadata)
resp := lookupResultError(err, emptyMetadata)
return resp.Resp, resp.Err
}

Expand Down
7 changes: 6 additions & 1 deletion internal/graph/lookupsubjects.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ func (cl *ConcurrentLookupSubjects) lookupDirectSubjects(
if it.Err() != nil {
return it.Err()
}
if tpl.Caveat != nil {
return NewUnimplementedErr(fmt.Errorf("cannot evaluate caveated relationships"))
}

if tpl.Subject.Namespace == req.SubjectRelation.Namespace &&
tpl.Subject.Relation == req.SubjectRelation.Relation {
Expand Down Expand Up @@ -198,7 +201,9 @@ func (cl *ConcurrentLookupSubjects) lookupViaTupleToUserset(
if it.Err() != nil {
return it.Err()
}

if tpl.Caveat != nil {
return NewUnimplementedErr(fmt.Errorf("cannot evaluate caveated relationships"))
}
toDispatchByTuplesetType.Add(tpl.Subject)
}

Expand Down
3 changes: 3 additions & 0 deletions internal/graph/reachableresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,9 @@ func (crr *ConcurrentReachableResources) chunkedRedispatch(it datastore.Relation
break
}

if tpl.Caveat != nil {
return NewUnimplementedErr(fmt.Errorf("cannot evaluate caveated relationships"))
}
resourceIdsFound = append(resourceIdsFound, tpl.ResourceAndRelation.ObjectId)
}

Expand Down
2 changes: 2 additions & 0 deletions internal/services/v1/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ func rewriteError(ctx context.Context, err error) error {
case errors.As(err, &graph.ErrAlwaysFail{}):
log.Ctx(ctx).Err(err).Msg("received internal error")
return status.Errorf(codes.Internal, "internal error: %s", err)
case errors.As(err, &graph.ErrUnimplemented{}):
return status.Errorf(codes.Unimplemented, "%s", err)

default:
log.Ctx(ctx).Err(err).Msg("received unexpected error")
Expand Down
55 changes: 48 additions & 7 deletions internal/services/v1/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import (
"context"
"fmt"

"github.com/authzed/spicedb/pkg/tuple"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb"

"github.com/authzed/authzed-go/pkg/requestmeta"

"github.com/authzed/authzed-go/pkg/responsemeta"
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/jzelinskie/stringz"
Expand All @@ -24,12 +26,20 @@ import (
"github.com/authzed/spicedb/pkg/middleware/consistency"
core "github.com/authzed/spicedb/pkg/proto/core/v1"
dispatch "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
"github.com/authzed/spicedb/pkg/tuple"
)

const maxCaveatContextBytes = 4096

func (ps *permissionServer) CheckPermission(ctx context.Context, req *v1.CheckPermissionRequest) (*v1.CheckPermissionResponse, error) {
atRevision, checkedAt := consistency.MustRevisionFromContext(ctx)
ds := datastoremw.MustFromContext(ctx).SnapshotReader(atRevision)

caveatContext, err := getCaveatContext(ctx, req.Context)
if err != nil {
return nil, err
}

// Perform our preflight checks in parallel
errG, checksCtx := errgroup.WithContext(ctx)
errG.Go(func() error {
Expand Down Expand Up @@ -71,7 +81,7 @@ func (ps *permissionServer) CheckPermission(ctx context.Context, req *v1.CheckPe
ObjectId: req.Subject.Object.ObjectId,
Relation: normalizeSubjectRelation(req.Subject),
},
CaveatContext: nil, // TODO(jschorr): get from the request
CaveatContext: caveatContext,
AtRevision: atRevision,
MaximumDepth: ps.config.MaximumAPIDepth,
IsDebuggingEnabled: isDebuggingEnabled,
Expand Down Expand Up @@ -104,17 +114,21 @@ func (ps *permissionServer) CheckPermission(ctx context.Context, req *v1.CheckPe
return nil, rewriteError(ctx, err)
}

// TODO(jschorr): Support caveated response here
var partialCaveat *v1.PartialCaveatInfo
permissionship := v1.CheckPermissionResponse_PERMISSIONSHIP_NO_PERMISSION
if cr.Membership == dispatch.ResourceCheckResult_MEMBER {
permissionship = v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION
} else if cr.Membership == dispatch.ResourceCheckResult_CAVEATED_MEMBER {
panic("caveats are not yet supported")
permissionship = v1.CheckPermissionResponse_PERMISSIONSHIP_CONDITIONAL_PERMISSION
partialCaveat = &v1.PartialCaveatInfo{
MissingRequiredContext: cr.MissingExprFields,
}
}

return &v1.CheckPermissionResponse{
CheckedAt: checkedAt,
Permissionship: permissionship,
CheckedAt: checkedAt,
Permissionship: permissionship,
PartialCaveatInfo: partialCaveat,
}, nil
}

Expand Down Expand Up @@ -301,6 +315,10 @@ func (ps *permissionServer) LookupResources(req *v1.LookupResourcesRequest, resp

ds := datastoremw.MustFromContext(ctx).SnapshotReader(atRevision)

if req.Context != nil {
return rewriteError(ctx, status.Error(codes.Unimplemented, "caveats not yet supported"))
}

// Perform our preflight checks in parallel
errG, checksCtx := errgroup.WithContext(ctx)
errG.Go(func() error {
Expand Down Expand Up @@ -374,6 +392,10 @@ func (ps *permissionServer) LookupSubjects(req *v1.LookupSubjectsRequest, resp v

ds := datastoremw.MustFromContext(ctx).SnapshotReader(atRevision)

if req.Context != nil {
return rewriteError(ctx, status.Error(codes.Unimplemented, "caveats not yet supported"))
}

// Perform our preflight checks in parallel
errG, checksCtx := errgroup.WithContext(ctx)
errG.Go(func() error {
Expand Down Expand Up @@ -459,3 +481,22 @@ func denormalizeSubjectRelation(relation string) string {
}
return relation
}

func getCaveatContext(ctx context.Context, caveatCtx *structpb.Struct) (map[string]any, error) {
var caveatContext map[string]any
if caveatCtx != nil {
if size := proto.Size(caveatCtx); size > maxCaveatContextBytes {
return nil, rewriteError(
ctx,
status.Errorf(
codes.InvalidArgument,
"request caveat context should have less than %d bytes but had %d",
maxCaveatContextBytes,
size,
),
)
}
caveatContext = caveatCtx.AsMap()
}
return caveatContext, nil
}
150 changes: 150 additions & 0 deletions internal/services/v1/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"math/rand"
"sort"
"testing"
"time"
Expand All @@ -19,6 +20,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/types/known/structpb"

"github.com/authzed/spicedb/internal/datastore/memdb"
v1svc "github.com/authzed/spicedb/internal/services/v1"
Expand Down Expand Up @@ -804,3 +806,151 @@ func TestLookupSubjects(t *testing.T) {
})
}
}

func TestCheckWithCaveats(t *testing.T) {
req := require.New(t)
conn, cleanup, _, revision := testserver.NewTestServer(req, testTimedeltas[0], memdb.DisableGC, true, tf.StandardDatastoreWithCaveatedData)
client := v1.NewPermissionsServiceClient(conn)
t.Cleanup(cleanup)

ctx := context.Background()

request := &v1.CheckPermissionRequest{
Consistency: &v1.Consistency{
Requirement: &v1.Consistency_AtLeastAsFresh{
AtLeastAsFresh: zedtoken.NewFromRevision(revision),
},
},
Resource: obj("document", "companyplan"),
Permission: "view",
Subject: sub("user", "owner", ""),
}

// caveat evaluated and returned false
var err error
request.Context, err = structpb.NewStruct(map[string]any{"secret": "incorrect_value"})
req.NoError(err)

checkResp, err := client.CheckPermission(ctx, request)
req.NoError(err)
req.Equal(v1.CheckPermissionResponse_PERMISSIONSHIP_NO_PERMISSION, checkResp.Permissionship)

// caveat evaluated and returned true
request.Context, err = structpb.NewStruct(map[string]any{"secret": "1234"})
req.NoError(err)

checkResp, err = client.CheckPermission(ctx, request)
req.NoError(err)
req.Equal(v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, checkResp.Permissionship)

// caveat evaluated but context variable was missing
request.Context = nil
checkResp, err = client.CheckPermission(ctx, request)
req.NoError(err)
req.Equal(v1.CheckPermissionResponse_PERMISSIONSHIP_CONDITIONAL_PERMISSION, checkResp.Permissionship)
req.EqualValues([]string{"secret"}, checkResp.PartialCaveatInfo.MissingRequiredContext)

// context exceeds length limit
request.Context, err = structpb.NewStruct(generateMap(64))
req.NoError(err)

_, err = client.CheckPermission(ctx, request)
grpcutil.RequireStatus(t, codes.InvalidArgument, err)
}

func TestLookupResourcesWithCaveats(t *testing.T) {
req := require.New(t)
conn, cleanup, _, revision := testserver.NewTestServer(req, testTimedeltas[0], memdb.DisableGC, true, tf.StandardDatastoreWithCaveatedData)
client := v1.NewPermissionsServiceClient(conn)
t.Cleanup(cleanup)

ctx := context.Background()

request := &v1.LookupResourcesRequest{
Consistency: &v1.Consistency{
Requirement: &v1.Consistency_AtLeastAsFresh{
AtLeastAsFresh: zedtoken.NewFromRevision(revision),
},
},
ResourceObjectType: "document",
Permission: "view",
Subject: sub("user", "owner", ""),
}

// caveat support is not implemented yet - forwarding a context returns gRPC error unimplemented
var err error
request.Context, err = structpb.NewStruct(map[string]any{"secret": "incorrect_value"})
req.NoError(err)

cli, err := client.LookupResources(ctx, request)
req.NoError(err)

_, err = cli.Recv()
grpcutil.RequireStatus(t, codes.Unimplemented, err)

// without caveat context the operation fails if caveated relationships are evaluated in the graph
request.Context = nil
cli, err = client.LookupResources(ctx, request)
req.NoError(err)

_, err = cli.Recv()
grpcutil.RequireStatus(t, codes.Unimplemented, err)
}

func TestLookupSubjectsWithCaveats(t *testing.T) {
req := require.New(t)
conn, cleanup, _, revision := testserver.NewTestServer(req, testTimedeltas[0], memdb.DisableGC, true, tf.StandardDatastoreWithCaveatedData)
client := v1.NewPermissionsServiceClient(conn)
t.Cleanup(cleanup)

ctx := context.Background()

request := &v1.LookupSubjectsRequest{
Consistency: &v1.Consistency{
Requirement: &v1.Consistency_AtLeastAsFresh{
AtLeastAsFresh: zedtoken.NewFromRevision(revision),
},
},
Resource: obj("document", "companyplan"),
Permission: "view",
SubjectObjectType: "user",
}

// caveat support is not implemented yet - forwarding a context returns gRPC error unimplemented
var err error
request.Context, err = structpb.NewStruct(map[string]any{"secret": "incorrect_value"})
req.NoError(err)

cli, err := client.LookupSubjects(ctx, request)
req.NoError(err)

_, err = cli.Recv()
grpcutil.RequireStatus(t, codes.Unimplemented, err)

// without caveat context the operation fails if caveated relationships are evaluated in the graph
request.Context = nil
cli, err = client.LookupSubjects(ctx, request)
req.NoError(err)

_, err = cli.Recv()
grpcutil.RequireStatus(t, codes.Unimplemented, err)
}

func generateMap(length int) map[string]any {
output := make(map[string]any, length)
for i := 0; i < length; i++ {
random := randString(32)
output[random] = random
}
return output
}

var randInput = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")

func randString(length int) string {
b := make([]rune, length)
for i := range b {
b[i] = randInput[rand.Intn(len(randInput))] //nolint:gosec
}
return string(b)
}
Loading

0 comments on commit 97069de

Please sign in to comment.