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

ReBAC: Updating user's entitlements fails silently #1419

Open
huwshimi opened this issue Nov 5, 2024 · 5 comments
Open

ReBAC: Updating user's entitlements fails silently #1419

huwshimi opened this issue Nov 5, 2024 · 5 comments

Comments

@huwshimi
Copy link
Contributor

huwshimi commented Nov 5, 2024

When updating a user's entitlements: PATCH /identities/{id}/entitlements there is no response body and no errors are returned.

For example the following gets a 200 OK response but does not create the relation:

{
  "patches": [
    {
      "entitlement": {
        "entity_type": "cloud",
        "entitlement": "administrator",
        "entity_id": "localhost"
      },
      "op": "add"
    }
  ]
}

But this succeeds:

{
  "patches": [
    {
      "entitlement": {
        "entity_type": "controller",
        "entitlement": "administrator",
        "entity_id": "d61369f7-7953-4a99-870a-4d046038fddc"
      },
      "op": "add"
    }
  ]
}
@huwshimi huwshimi changed the title Updating user's entitlements fails silently ReBAC: Updating user's entitlements fails silently Nov 5, 2024
@kian99
Copy link
Contributor

kian99 commented Nov 6, 2024

I've made a test for this but I can't seem to replicate it.
The test is an integration test but calls the handler directly in code (I haven't tried an actual HTTP call).
But it seems to work, I grant a user access to a "test-cloud" and then verify they have the expected access.

Why do you suggest the first block doesn't actually create the relation? Is it not showing up in one of the other calls?

@huwshimi
Copy link
Contributor Author

huwshimi commented Nov 6, 2024

Ah interesting, if I try and create that same relation again it says it already exists, so that call is succeeding.

However, the created relation is not included in the entitlements call e.g. /identities/{id}/entitlements.

{
    "_links": {
        "next": {
            "href": "/rebac/v1/identities/[email protected]/entitlements?nextToken=eyJraW5kIjoiY2xvdWQiLCJ0b2tlbiI6IiJ9"
        }
    },
    "_meta": {
        "pageToken": "",
        "size": 1
    },
    "data": [
        {
            "entitlement": "administrator",
            "entity_id": "3217dbc9-8ea9-4381-9e97-01eab0b3f6bb",
            "entity_type": "controller"
        }
    ],
    "message": "",
    "status": 200
}

@kian99
Copy link
Contributor

kian99 commented Nov 12, 2024

Digging into this further, I can see why this is happening. In our implementation for pagination of OpenFGA results, we realised it is not trivial to fetch all the relations of a user. What OpenFGA offers is the ability to fetch the relations for a specific user + specific object. So "user" + "cloud" or "user" + "controller".

You'll notice the response from /identities/{id}/entitlements includes a next page with a "nextToken", in our implementation, we page over the combinations of "user" + and so a page can be empty but the next page might not be.

I think our expectation was to document this and have the client page over the results until they've either received as many results as they want to show in a page or until the nextPage token is empty.

Fortunately, if this behaviour is not aligned with other backend implementations of the rebac-admin-ui we can fairly easily do this pagination on the backend and return the expected result. Let me know what you think @huwshimi.

@huwshimi
Copy link
Contributor Author

Thanks for figuring that out @kian99. It would be great if this could be handled by the backend if that's OK.

The future plan will be to update these endpoints with filtering, page-size etc. so it would be great for the backend to handle as much of that as possible.

@SimoneDutto
Copy link
Contributor

Hi @huwshimi , we've handled the pagination from the backend and it is now merged in v3. Let us know if now everything works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants