From ce86bdc9fff43b66bc928191766b76d30aac99bc Mon Sep 17 00:00:00 2001 From: Andrei Mihu Date: Thu, 7 Mar 2019 20:14:04 +0000 Subject: [PATCH] Reliably allow storage listings both across users and for individual users. --- CHANGELOG.md | 2 ++ server/api_storage.go | 4 ++-- server/core_storage.go | 44 +++++++++++++++++++++++++----------- server/runtime_go_nakama.go | 6 ++--- server/runtime_lua_nakama.go | 4 ++-- tests/core_storage_test.go | 6 ++--- 6 files changed, 43 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00d6407d9..ab24f897b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [keep a changelog](http://keepachangelog.com) and this pr - New runtime constants representing storage permissions. - New runtime function to programmatically delete user accounts. - Allow multiple config files to be read at startup and merged into a final server configuration. +- Storage listing operations can now disambiguate between listing system-owned data and listing all data. ### Changed - Default maximum database connection lifetime is now 1 hour. @@ -17,6 +18,7 @@ The format is based on [keep a changelog](http://keepachangelog.com) and this pr - Made Go and Lua runtime startup log messages consistent. - All schema and query statements that use the '1970-01-01 00:00:00' constant now specify UTC timezone. - Storage write error message now correctly indicates that values must be encoded JSON objects. +- Storage listing operations now treat empty owner IDs as listing across all data rather than system-owned data. ### Fixed - CRON expressions for leaderboard and tournament resets now allow concurrent processing. diff --git a/server/api_storage.go b/server/api_storage.go index 58440c604..91ca6aa05 100644 --- a/server/api_storage.go +++ b/server/api_storage.go @@ -60,13 +60,13 @@ func (s *ApiServer) ListStorageObjects(ctx context.Context, in *api.ListStorageO limit = int(in.GetLimit().Value) } - userID := uuid.Nil + var userID *uuid.UUID if in.GetUserId() != "" { uid, err := uuid.FromString(in.GetUserId()) if err != nil { return nil, status.Error(codes.InvalidArgument, "Invalid user ID - make sure user ID is a valid UUID.") } - userID = uid + userID = &uid } storageObjectList, code, listingError := StorageListObjects(ctx, s.logger, s.db, caller, userID, in.GetCollection(), limit, in.GetCursor()) diff --git a/server/core_storage.go b/server/core_storage.go index d319ef5ca..ff1b6bf4e 100644 --- a/server/core_storage.go +++ b/server/core_storage.go @@ -39,7 +39,7 @@ type storageCursor struct { Read int32 } -func StorageListObjects(ctx context.Context, logger *zap.Logger, db *sql.DB, caller uuid.UUID, ownerID uuid.UUID, collection string, limit int, cursor string) (*api.StorageObjectList, codes.Code, error) { +func StorageListObjects(ctx context.Context, logger *zap.Logger, db *sql.DB, caller uuid.UUID, ownerID *uuid.UUID, collection string, limit int, cursor string) (*api.StorageObjectList, codes.Code, error) { var sc *storageCursor = nil if cursor != "" { sc = &storageCursor{} @@ -56,19 +56,28 @@ func StorageListObjects(ctx context.Context, logger *zap.Logger, db *sql.DB, cal var result *api.StorageObjectList var resultErr error + if caller == uuid.Nil { - // disregard permissions - result, resultErr = StorageListObjectsUser(ctx, logger, db, true, ownerID, collection, limit, cursor, sc) - } else if ownerID == uuid.Nil { - // not authoritative but trying to list global data from context of a user. - result, resultErr = StorageListObjectsPublicRead(ctx, logger, db, collection, limit, cursor, sc) + // Call from the runtime. + if ownerID == nil { + // List storage regardless of user. + // TODO + result, resultErr = StorageListObjectsAll(ctx, logger, db, true, collection, limit, cursor, sc) + } else { + // List for a particular user ID. + result, resultErr = StorageListObjectsUser(ctx, logger, db, true, *ownerID, collection, limit, cursor, sc) + } } else { - if caller == ownerID { - // trying to list own user data - result, resultErr = StorageListObjectsUser(ctx, logger, db, false, ownerID, collection, limit, cursor, sc) + // Call from a client. + if ownerID == nil { + // List publicly readable storage regardless of owner. + result, resultErr = StorageListObjectsAll(ctx, logger, db, false, collection, limit, cursor, sc) + } else if o := *ownerID; caller == o { + // User listing their own data. + result, resultErr = StorageListObjectsUser(ctx, logger, db, false, o, collection, limit, cursor, sc) } else { - // trying to list someone else's data - result, resultErr = StorageListObjectsPublicReadUser(ctx, logger, db, ownerID, collection, limit, cursor, sc) + // User listing someone else's data. + result, resultErr = StorageListObjectsPublicReadUser(ctx, logger, db, o, collection, limit, cursor, sc) } } @@ -79,7 +88,7 @@ func StorageListObjects(ctx context.Context, logger *zap.Logger, db *sql.DB, cal return result, codes.OK, nil } -func StorageListObjectsPublicRead(ctx context.Context, logger *zap.Logger, db *sql.DB, collection string, limit int, cursor string, storageCursor *storageCursor) (*api.StorageObjectList, error) { +func StorageListObjectsAll(ctx context.Context, logger *zap.Logger, db *sql.DB, authoritative bool, collection string, limit int, cursor string, storageCursor *storageCursor) (*api.StorageObjectList, error) { cursorQuery := "" params := []interface{}{collection, limit} if storageCursor != nil { @@ -87,11 +96,20 @@ func StorageListObjectsPublicRead(ctx context.Context, logger *zap.Logger, db *s params = append(params, storageCursor.Key, storageCursor.UserID) } - query := ` + var query string + if authoritative { + query = ` +SELECT collection, key, user_id, value, version, read, write, create_time, update_time +FROM storage +WHERE collection = $1` + cursorQuery + ` +LIMIT $2` + } else { + query = ` SELECT collection, key, user_id, value, version, read, write, create_time, update_time FROM storage WHERE collection = $1 AND read = 2` + cursorQuery + ` LIMIT $2` + } var objects *api.StorageObjectList err := ExecuteRetryable(func() error { diff --git a/server/runtime_go_nakama.go b/server/runtime_go_nakama.go index a7a6f1e81..c9a7f3ca0 100644 --- a/server/runtime_go_nakama.go +++ b/server/runtime_go_nakama.go @@ -1016,13 +1016,13 @@ func (n *RuntimeGoNakamaModule) WalletLedgerList(ctx context.Context, userID str } func (n *RuntimeGoNakamaModule) StorageList(ctx context.Context, userID, collection string, limit int, cursor string) ([]*api.StorageObject, string, error) { - uid := uuid.Nil - var err error + var uid *uuid.UUID if userID != "" { - uid, err = uuid.FromString(userID) + u, err := uuid.FromString(userID) if err != nil { return nil, "", errors.New("expects an empty or valid user id") } + uid = &u } objectList, _, err := StorageListObjects(ctx, n.logger, n.db, uuid.Nil, uid, collection, limit, cursor) diff --git a/server/runtime_lua_nakama.go b/server/runtime_lua_nakama.go index fdab8e004..93ca5da0d 100644 --- a/server/runtime_lua_nakama.go +++ b/server/runtime_lua_nakama.go @@ -3487,14 +3487,14 @@ func (n *RuntimeLuaNakamaModule) storageList(l *lua.LState) int { limit := l.CheckInt(3) cursor := l.OptString(4, "") - userID := uuid.Nil + var userID *uuid.UUID if userIDString != "" { uid, err := uuid.FromString(userIDString) if err != nil { l.ArgError(1, "expects empty or a valid user ID") return 0 } - userID = uid + userID = &uid } objectList, _, err := StorageListObjects(l.Context(), n.logger, n.db, uuid.Nil, userID, collection, limit, cursor) diff --git a/tests/core_storage_test.go b/tests/core_storage_test.go index 3153f1084..3fcac2488 100644 --- a/tests/core_storage_test.go +++ b/tests/core_storage_test.go @@ -1592,7 +1592,7 @@ func TestStorageListRuntimeUser(t *testing.T) { assert.NotNil(t, acks, "acks was nil") assert.Len(t, acks.Acks, 3, "acks length was not 3") - list, code, err := server.StorageListObjects(context.Background(), logger, db, uuid.Nil, uid, "testcollection", 10, "") + list, code, err := server.StorageListObjects(context.Background(), logger, db, uuid.Nil, &uid, "testcollection", 10, "") assert.Nil(t, err, "err was not nil") assert.Equal(t, codes.OK, code, "code was not OK") @@ -1639,7 +1639,7 @@ func TestStorageListPipelineUserSelf(t *testing.T) { assert.NotNil(t, acks, "acks was nil") assert.Len(t, acks.Acks, 3, "acks length was not 3") - list, code, err := server.StorageListObjects(context.Background(), logger, db, uid, uid, collection, 10, "") + list, code, err := server.StorageListObjects(context.Background(), logger, db, uid, &uid, collection, 10, "") assert.Nil(t, err, "err was not nil") assert.Equal(t, codes.OK, code, "code was not OK") @@ -1688,7 +1688,7 @@ func TestStorageListPipelineUserOther(t *testing.T) { assert.NotNil(t, acks, "acks was nil") assert.Len(t, acks.Acks, 3, "acks length was not 3") - values, code, err := server.StorageListObjects(context.Background(), logger, db, uuid.Must(uuid.NewV4()), uid, collection, 10, "") + values, code, err := server.StorageListObjects(context.Background(), logger, db, uuid.Must(uuid.NewV4()), &uid, collection, 10, "") assert.Nil(t, err, "err was not nil") assert.Equal(t, codes.OK, code, "code was not OK") -- GitLab