From 73a429e0ff1e6f312a696385ce2ca191a0ca0d16 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 28 Aug 2023 16:52:25 -0400 Subject: [PATCH] Have invalid cursors return a proper error Fixes #1502 --- internal/services/v1/relationships_test.go | 33 ++++++++++++++++ pkg/cursor/cursor.go | 28 +++++-------- pkg/cursor/errors.go | 46 ++++++++++++++++++++++ 3 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 pkg/cursor/errors.go diff --git a/internal/services/v1/relationships_test.go b/internal/services/v1/relationships_test.go index f608d0254a..0b5d9539ba 100644 --- a/internal/services/v1/relationships_test.go +++ b/internal/services/v1/relationships_test.go @@ -1383,6 +1383,39 @@ func TestReadRelationshipsWithTimeout(t *testing.T) { } } +func TestReadRelationshipsInvalidCursor(t *testing.T) { + require := require.New(t) + + conn, cleanup, _, revision := testserver.NewTestServer(require, 0, memdb.DisableGC, true, tf.StandardDatastoreWithData) + client := v1.NewPermissionsServiceClient(conn) + t.Cleanup(cleanup) + + stream, err := client.ReadRelationships(context.Background(), &v1.ReadRelationshipsRequest{ + Consistency: &v1.Consistency{ + Requirement: &v1.Consistency_AtLeastAsFresh{ + AtLeastAsFresh: zedtoken.MustNewFromRevision(revision), + }, + }, + RelationshipFilter: &v1.RelationshipFilter{ + ResourceType: "folder", + OptionalResourceId: "auditors", + OptionalRelation: "viewer", + OptionalSubjectFilter: &v1.SubjectFilter{ + SubjectType: "user", + OptionalSubjectId: "jeshk", + }, + }, + OptionalLimit: 42, + OptionalCursor: &v1.Cursor{Token: "someinvalidtoken"}, + }) + require.NoError(err) + + _, err = stream.Recv() + require.Error(err) + require.ErrorContains(err, "error decoding cursor") + grpcutil.RequireStatus(t, codes.InvalidArgument, err) +} + func readOfType(require *require.Assertions, resourceType string, client v1.PermissionsServiceClient, token *v1.ZedToken) map[string]struct{} { got := make(map[string]struct{}) stream, err := client.ReadRelationships(context.Background(), &v1.ReadRelationshipsRequest{ diff --git a/pkg/cursor/cursor.go b/pkg/cursor/cursor.go index d48159d401..38df2353ee 100644 --- a/pkg/cursor/cursor.go +++ b/pkg/cursor/cursor.go @@ -13,25 +13,13 @@ import ( "github.com/authzed/spicedb/pkg/spiceerrors" ) -// Public facing errors -const ( - errEncodeError = "error encoding cursor: %w" - errDecodeError = "error decoding cursor: %w" -) - -// ErrNilCursor is returned as the base error when nil is provided as the -// cursor argument to Decode -var ErrNilCursor = errors.New("cursor pointer was nil") - -// ErrHashMismatch is returned as the base error when a mismatching hash was given to the decoder. -var ErrHashMismatch = errors.New("the cursor provided does not have the same arguments as the original API call; please ensure you are making the same API call, with the exact same parameters (besides the cursor)") - // Encode converts a decoded cursor to its opaque version. func Encode(decoded *impl.DecodedCursor) (*v1.Cursor, error) { marshalled, err := decoded.MarshalVT() if err != nil { - return nil, fmt.Errorf(errEncodeError, err) + return nil, NewInvalidCursorErr(fmt.Errorf(errEncodeError, err)) } + return &v1.Cursor{ Token: base64.StdEncoding.EncodeToString(marshalled), }, nil @@ -40,17 +28,19 @@ func Encode(decoded *impl.DecodedCursor) (*v1.Cursor, error) { // Decode converts an encoded cursor to its decoded version. func Decode(encoded *v1.Cursor) (*impl.DecodedCursor, error) { if encoded == nil { - return nil, fmt.Errorf(errDecodeError, ErrNilCursor) + return nil, NewInvalidCursorErr(errors.New("cursor pointer was nil")) } decodedBytes, err := base64.StdEncoding.DecodeString(encoded.Token) if err != nil { - return nil, fmt.Errorf(errDecodeError, err) + return nil, NewInvalidCursorErr(fmt.Errorf(errDecodeError, err)) } + decoded := &impl.DecodedCursor{} if err := decoded.UnmarshalVT(decodedBytes); err != nil { - return nil, fmt.Errorf(errDecodeError, err) + return nil, NewInvalidCursorErr(fmt.Errorf(errDecodeError, err)) } + return decoded, nil } @@ -87,11 +77,11 @@ func DecodeToDispatchCursor(encoded *v1.Cursor, callAndParameterHash string) (*d v1decoded := decoded.GetV1() if v1decoded == nil { - return nil, ErrNilCursor + return nil, NewInvalidCursorErr(ErrNilCursor) } if v1decoded.CallAndParametersHash != callAndParameterHash { - return nil, ErrHashMismatch + return nil, NewInvalidCursorErr(ErrHashMismatch) } return &dispatch.Cursor{ diff --git a/pkg/cursor/errors.go b/pkg/cursor/errors.go new file mode 100644 index 0000000000..afb47628ae --- /dev/null +++ b/pkg/cursor/errors.go @@ -0,0 +1,46 @@ +package cursor + +import ( + "errors" + + v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/authzed/spicedb/pkg/spiceerrors" +) + +// Public facing errors +const ( + errEncodeError = "error encoding cursor: %w" + errDecodeError = "error decoding cursor: %w" +) + +// ErrNilCursor is returned as the base error when nil is provided as the +// cursor argument to Decode +var ErrNilCursor = errors.New("cursor pointer was nil") + +// ErrHashMismatch is returned as the base error when a mismatching hash was given to the decoder. +var ErrHashMismatch = errors.New("the cursor provided does not have the same arguments as the original API call; please ensure you are making the same API call, with the exact same parameters (besides the cursor)") + +// ErrInvalidCursor occurs when a cursor could not be decoded. +type ErrInvalidCursor struct { + error +} + +// GRPCStatus implements retrieving the gRPC status for the error. +func (err ErrInvalidCursor) GRPCStatus() *status.Status { + return spiceerrors.WithCodeAndDetails( + err, + codes.InvalidArgument, + spiceerrors.ForReason( + v1.ErrorReason_ERROR_REASON_INVALID_CURSOR, + nil, + ), + ) +} + +// NewInvalidCursorErr creates and returns a new invalid cursor error. +func NewInvalidCursorErr(err error) error { + return ErrInvalidCursor{err} +}