Make project context (vlab, proj) ids optional

[eleftherioszisis]

In most endpoints a user is required to provide a project and virtual lab id in the header of the request. This project context is then validated against their user groups and used to constrain queries. The rest of their user groups is ignored in the queries.

However, that context imposes a constraint (on a specific proj id) that may not be always desirable. Therefore, I propose to make the project context optional for all endpoints, except for the create ones that require it to register the entity.

To maintain backwards compatibility and stricter access to resources, when a project context is provided the behavior should remain as before, constraining all queries to the specific project context.

If not provided then the user's groups, as obtained from keycloak, should be used to determine if a user has access to a resource. Naturally, this makes the behavior of the endpoints more permissive allowing the user to perform queries with respect of all their project ids.

TLDR:

• If a user passes vlab/proj id pair in the header things will work as they used to constraining the queries into that project.
• If a user passes just their token, then the user_project_ids in the token will be used in the queries.

Draft here: #370

@jdcourcol @mgeplf @GianlucaFicarelli

[GianlucaFicarelli]

Note that this is a breaking change for retrieving public resources because to retrieve public entities:

• now the frontend doesn't pass virtual-lab-id/project-id in the header
• with this change, the frontend should specify in the requests the parameter authorized_public=true as added in https://github.com/openbraininstitute/prod-platform-architecture/issues/125 when retrieving entities. However, this parameter must not be specified when retrieving global resources (species, strain, brain_region...) because they are public by definition and they don't contain this attribute.

I think the main use case for this proposal is to allow the clients to retrieve all their private entities across multiple projects in a single request. Can this be useful in the new UI or in some planned functionality? Are there other use cases?

Side note just for clarity: the retrieval of resources that are public OR belonging to a specific project cannot be expressed with a regular AND filter, but this isn't going to change with this proposal.

[jdcourcol]

This was not motivated by a use case. It found it very strange that:

• I have to pass virtual-lab-id and project-id to update or delete data : the server knows what is the project id for that entity
• I have to pass virtual-lab-id when the project-id is enough to create data (and we store only that)

[GianlucaFicarelli]

In my previous comment I was considering only the get_many endpoint, but it's clearer if we consider each type of endpoint that we have now:

• read_many
• create_one
• read_one/{id}
• update_one/{id}
• delete_one/{id}
• admin_read_one/{id}
• admin_update_one/{id}
• admin_delete_one/{id}

In the endpoints above:

• The read_many can return in a single call:
  • public entities belonging to any project, or
  • public OR private entities, both belonging to a specific project, or
  • public entities belonging to any project, OR private entities belonging to a specific project: do we still need that?
• In the create_one the project_id must be passed in some way (in the headers or in the payload).
• For the other endpoints, we can avoid to specify the project_id because it's determined by the entity_id.

The virtual-lab-id is currently used only when creating an asset, because it's part of the prefix used in S3. We can decide to remove the virtual-lab-id from the prefix, or keep it as is but determine it from the project-id (from the keycloak user info, or from virtual-lab-manager)

The concept of project context was initially meant to simplify the logic and prevent mistakes, since the user was initially assumed to be always logged in a specific project, but this can be changed.

[jdcourcol]

The virtual-lab-id is currently used only when creating an asset, because it's part of the prefix used in S3. We can decide to remove the virtual-lab-id from the prefix, or keep it as is but determine it from the project-id (from the keycloak user info, or from virtual-lab-manager)

The concept of project context was initially meant to simplify the logic and prevent mistakes, since the user was initially assumed to be always logged in a specific project, but this can be changed.

The issue is that we cannot really trust the context as it is set client side. it can always set a virtual_lab_id and a project_id that do not relate to each other.
The only relevant information is the project_id. From that, we should find the virtual_lab_id if needed.

[eleftherioszisis]

The only relevant information is the project_id. From that, we should find the virtual_lab_id if needed.

If needed, on server side when we populate the context with the project_id the vlab_id could also be extracted from the keycloak group without additional information:

/proj/a98b7abc-fc46-4700-9e3d-37137812c730/0dbced5f-cc3d-488a-8c7f-cfb8ea039dc6/admin'

[jdcourcol]

The read_many can return in a single call:

public entities belonging to any project, or
public OR private entities, both belonging to a specific project, or
public entities belonging to any project, OR private entities belonging to a specific project: do we still need that?

For these, I feel that it is easier to manage the "scope" as a header (and only the project_id is needed) than as a filter.
The first query is used by explore ("public tab")
The second query is used by explore ("project tab")
The third one looks like a union of the first 2 no ? I feel that it is what I would use when using entitysdk.

[GianlucaFicarelli]

The issue is that we cannot really trust the context as it is set client side. it can always set a virtual_lab_id and a project_id that do not relate to each other.

virtual_lab_id and a project_id are validated with the userinfo returned by keycloak, so they can be trusted after that.
However, partially related, see also the issue with projects created by users invited as admin in https://github.com/openbraininstitute/prod-virtual-lab/issues/203#issuecomment-3337168648. In that case, the new project_id isn't included in the userinfo of the initial admin.

[eleftherioszisis]

To summarize, do we agree with the following behavior?

General

• Make virtual_lab_id optional and deduce it from userinfo project_id if not provided (to maintain how assets work for now)

Endpoints

create_one

• project_id is mandatory in header

read_one

• If project_id in header, constrain to private entities in that project or all public ones
• If not, constrain to private entities in user_project_ids or all public ones

read_many

• If project_id in header, constrain to private entities in that project or all public ones
• If not, constrain to private entities in user_project_ids or all public ones
• To retrieve exclusively public resources: {"authorized_public": True}

update_one

• If project_id in header, constrain to private entities in that project
• If not, constrain to private entities in user_project_ids

delete_one

• If project_id in header, constrain to private entities in that project
• If not, constrain to private entities in user_project_ids

[jdcourcol]

for update_one and delete_one, is there any good reason to have the project_id in the header ?
I think I agree with the proposal. (however, it is not a priority over task manager)

[eleftherioszisis]

for update_one and delete_one, is there any good reason to have the project_id in the header ?

It would allow in the client to intuitively constrain all requests within a specific project. For example, one could do with entitysdk:

client = Client(..., project_context=project_context)

and expect that all endpoints will respect that context and ensure that we don't perform actions to different projects by mistake.

[eleftherioszisis]

When uploading assets a header project context is required. However, we could add in the proposal the possibility to be able to upload an asset to any entity as long as the user has access to the project of the entity.

The main issue with an optional context in this case is that the virtual_lab_id is not readily available. However, it can still be deduced via the user_context (that allows to get vlab from proj id using keycloak groups after merging #390) and the project_id from the entity row.

[jdcourcol]

The proj_id is on the entity. Then the keycloak group tells you if you have permission for that project. If you need the vlab id, you would ask the virtual lab api.

[eleftherioszisis]

Asset creation requires the vlab_id to form the s3 path. Why do we need to burden entitycore with an additional dependency when we already have the keycloak groups?