Response from storageList does not respect permissions!

Documentation
1

Description

Calling nk.storageList(null, collection_name, 10) returns objects that should not be readable. Private objects are returned without a user ID, when only public objects should be returned.

Steps to Reproduce

  • Create various storage objects with permissionRead set to 0, 1, 2
  • Call nk.storageList(null, collection_name, 10)
  • Inspect the results and see that all objects are returned

Expected Result

Only objects that have public read permission or belong to the current user should be returned

Actual Result

I get a response like this:

[
    {
        "collection": "cards",
        "version": "2488e59d6392a3412a72e21d5c9b3d3f",
        "permissionWrite": 1,
        "createTime": 1693909271,
        "updateTime": 1693910430,
        "key": "6be82de6-2dd3-46da-8350-5f1040cd5b82",
        "permissionRead": 0,
        "value": {
            "text": "This card is only user 1"
        },
        "userId": "a4ff99bb-f481-46ef-ba22-1e78406e6473"
    },
    {
        "permissionRead": 0,
        "permissionWrite": 1,
        "createTime": 1693910254,
        "value": {
            "text": "This card is only user 2"
        },
        "userId": "b2da6a30-4bcc-4184-8e53-824d0302ebc1",
        "version": "db0efeee4f01f50c936994f39a761bda",
        "updateTime": 1693910414,
        "key": "baf11449-9e7c-4cff-b46e-5ddde1250523",
        "collection": "cards"
    },
    {
        "permissionRead": 2,
        "createTime": 1693910191,
        "updateTime": 1693910191,
        "key": "8a703ab0-45f2-434f-9403-3432d7399dbb",
        "collection": "cards",
        "userId": "a4ff99bb-f481-46ef-ba22-1e78406e6473",
        "version": "d230ffb0a87e7b70b7072d740ffc422e",
        "permissionWrite": 1,
        "value": {
            "text": "This is a public card"
        }
    }
]

Context

Using JS RPC functions that I added to nakama-project-template.

Your Environment

  • Server version: 3.16.0+27ba93d3 running in Docker

Update: based on the documentation for storageList, instead of null, I should pass in an empty string for the user ID. But I tried that as well, and end up getting this error:

template_nk_backend   | {"level":"error","ts":"2023-09-07T14:49:05.948Z","caller":"server/runtime_javascript.go:551","msg":"JavaScript runtime function raised an uncaught exception","mode":"rpc","id":"get_cards","error":"TypeError: expects empty or valid user id at github.com/heroiclabs/nakama/v3/server.(*runtimeJavascriptNakamaModule).storageList.func1 (native)"}
2

Hello @dzso,

When the operation is called from the server runtime the permissions are not applied because the server should be able to access all objects, the first param of nk.storageList is to filter the records by their owner or list all records in the collection (if empty string or null).

We will add an additional param called callerID to the function to specify whether to filter the results based on the permissions or assume the caller is the system user.

In the meantime you can filter the results that shouldn’t be returned with some custom logic.

Best

3

Thanks for your response. I understand that the server itself can access all objects, but if a user is passed in as the first parameter of storageList, according to the documentation, it should only return that user’s objects. According to the documentation, the first parameter is userID;

User ID to list records for or “” (empty string) for public records.

So when the userID is empty, it should only return public records, and when it is passed, it should only return records for that user’s objects. I don’t think there’s a need to add callerID. The first parameter should be enough, it’s just not working as documented.

And yes, I can write custom logic to filter the results, but then it’s going to mess up the limit parameter, because I will request a certain number of objects, get a different number, and then the value of the cursor will be off by the number of items I filtered out. It makes a lot of complicated and error-prone logic on my end, rather than just having the userID of the storageList function return the expected items.

4

The documentation is incorrect and will be updated, if empty string is passed for the userID from the server runtime API then all records in the collection are returned, the current statement only holds true when storage listing is used from the client-facing API.

The additional callerID is required otherwise there’s no way to list all the storage object in a collection from the server regardless of access level.

I think that you need to keep using the same limit across pages and it’ll work fine, however you’re right - you’ll need additional logic to stitch together the filtered results into cohesive length pages, but it shouldn’t require much logic.

As mentioned the new param will be introduced to the runtime API in the next release so you won’t need this workaround.

Best.

5

How do you propose that I should be requesting storage listing from the client-facing API? I didn’t see a function for doing that on the client side, so right now, I am creating an RPC function that calls storageList so that I can get items from storage. Is there a built in client side way to call storageList instead? This is my attempt at the logic of stitching together results using an RPC function, but it seems to be pretty complicated when all I want to do is get a list of storage objects that the user has permission to see.

let rpcGetCardsLimit: nkruntime.RpcFunction = function (
  ctx: nkruntime.Context,
  logger: nkruntime.Logger,
  nk: nkruntime.Nakama,
  payload: string
): string {
  if (!ctx.userId) throw Error('No user ID in context');
  
  const args = JSON.parse(payload);
  const desiredLimit = args.limit || 10;
  let currentCursor = args.cursor || null;
  const fetchedCards: any[] = [];
  
  while (fetchedCards.length < desiredLimit) {
    const publicCards = nk.storageList(null, "cards", desiredLimit - fetchedCards.length, currentCursor);
    
    if (publicCards.objects) {
      for (const card of publicCards.objects) {
        if (card.userId === ctx.userId || card.permissionRead >= 2) {
          fetchedCards.push(card);
        }
        if (fetchedCards.length >= desiredLimit) {
          // Break out of the loop when the desired limit is reached
          break;
        }
      }
    }
    
    if (!publicCards.cursor) {
      // No more cards left in storage to fetch
      break;
    }
    // Update the cursor after checking the limit to ensure it points to the next item
    currentCursor = publicCards.cursor;
  }
  
  return JSON.stringify({
    items: fetchedCards,
    cursor: currentCursor
  });
};

And on the front end:

const [session, setSession] = useState<Session | null>(null);
const [errorMessages, setErrorMessages] = useState<string[]>([]);
const [cardsCursor, setCardsCursor] = useState<string | null>(null);
// ...
const getCards = async () => {
    try {
      if (!session) return;
      const rpcid = 'get_cards_limit';
      const response = await client.rpc(session, rpcid, { cursor: cardsCursor, limit: 3});
      console.log('get cards response', response);
      if (response.payload?.cursor) {
        setCardsCursor(response.payload.cursor);
      }
    } catch (error: any) {
      console.error('Nakama failed to get cards:', error);
      const message = `Failed to get cards: ${error.message}`;
      setErrorMessages([...errorMessages, message]);
    }
  }

This seems to work, but it is very tedious. Again, all I really want to do is get the results I expect from storageList(userId, collection, limit). Should be a one liner.

6

There’s a client facing API to list objects:

7

Thanks! That helps. I missed this in the documentation before. I don’t have the bandwidth to read the source code to try to reverse engineer the API. But the documentation does have an example here that I missed before: Heroic Labs Documentation | Collections

8

I’m running into another problem, and wondering the correct way to handle this.

I can get all of the objects owned by a user by doing this:

client.listStorageObjects(session, collectionName, session.user_id, 100);

And I can get all of the publicly visible objects by doing this:

client.listStorageObjects(session, collectionName, undefined, 100);

But I don’t see a way to get all of the objects owned by a user, plus all of the objects that are publicly visible. Yeah, I can do two separate queries, and combine them, though again I’m going to run into tedious issues with consistent pagination. Is there no simple way to get all of the objects visible to a user, both public and private?

9

A way to do it would be to use the storageRead API and provide the collection and keys of the objects you’d like to read.

Could you please elaborate on how you’ve modeled your data and what are you trying to achieve?

10

I don’t know the keys of the objects I’d like to read. That’s why I’m listing them. I have a collection called cards, that are just some simple objects with a couple fields. I have a page in my app that lists all of the cards visible by the logged in user, including the cards that they own, and it has pages, because the list of cards will be quite long. There is also the option for the user to filter the list just to the cards that they own, and not to see other public cards. But there isn’t really a use case for the user to see public cards without their own. This doesn’t seem to me like it should be a complicated use case.

11

If the list of available cards is a superset of the cards owned by the user, just grab all cards owned by the user first, then use client.listStorageObjects(session, collectionName, undefined, 100); to list all available cards and lookup any in the list of owned cards, if found just mark them as owned. It would be best to store all the cards owned by the player in a single storage object that points to the individual cards storage objects.

If it is not a superset and I misunderstood the data model you’ll have to please provide more information.

Hope this helps.

12

The problem is that the cards returned by client.listStorageObjects(session, collectionName, undefined, 100) is NOT a superset of cards owned by the user because it does not include private cards owned by the user. So I have to do client.listStorageObjects(session, collectionName, userId, 100) to get the private cards owned by the user, but that doesn’t include any public cards visible by the user. So neither is a superset of the other. I want to get a list of all cards visible to a user.

The data model is straightforward and simple. I believe it’s the most basic use case possible: I have one collection called cards each of those cards looks like this:

{
  "text": "This is a card"
}

And has read permissions set to 0, 1 or 2, and write permissions set to 0 or 1.

All I want to do is make a simple list of cards visible to the user. All I need in my app is an object like this:

[
  { "text": "Private card owned by user" },
  { "text": "Public card owned by user" },
  { "text": "Public card owned by another user" }
]

This seems like a simple use case for me. Do I really have to combine two separate queries to do it? That breaks pagination, and requires managing two separate cursors on the front end.

13

FWIW, I ended up implementing a frontend solution to manage merging both queries. It still needs a little bit of work to support pagination, but it’s a start. This is TypeScript/React, using react-query and lodash:

function useCursors() {
  const [privateCursor, setPrivateCursor] = useState<string|undefined>(undefined);
  const [publicCursor, setPublicCursor] = useState<string|undefined>(undefined);
  return {
    privateCursor,
    publicCursor,
    setPrivateCursor,
    setPublicCursor
  };
}

function useStorageObjects(collectionName: string, session: Session | null) {
  const {
    privateCursor,
    publicCursor,
    setPrivateCursor,
    setPublicCursor
  } = useCursors();
  const queryResults = useQuery({
    queryKey: [collectionName, { session }],
    queryFn: async ({ queryKey }) => {
      const session = get(queryKey, '1.session') as unknown as Session;
      if (!session) return {};
      const [privateResponse, publicResponse] = await Promise.all([
        client.listStorageObjects(session, collectionName, session.user_id ?? undefined, 100, privateCursor),
        client.listStorageObjects(session, collectionName, undefined, 100, publicCursor)
      ]);
      const combinedObjects = uniqBy([...privateResponse.objects, ...publicResponse.objects], 'key');
      return {
        objects: combinedObjects,
        privateCursor: privateResponse.cursor,
        publicCursor: publicResponse.cursor
      };
    }
  });
  useEffect(() => {
    if (queryResults.isSuccess) {
      setPrivateCursor(queryResults.data.privateCursor);
      setPublicCursor(queryResults.data.publicCursor);
    }
  }, [queryResults]);
  return queryResults;
}

const cards = useStorageObjects('cards', session);

console.log('Cards visible to the user:', cards.data.objects);

That’s a lot of boilerplate for the front end. I think fetching a list of objects visible to the user should be a one-liner.