Add bulk permission management and copy functionality via API

[flooobauer]

In our hosted instance, we are creating objects with a bot account for specific users because we can't upload them directly from the user's account. Nevertheless we'd like to use the same permissions from the sample on the process we just performed on it. While there's currently just the option to read and set each permission type (users, group, projects, ...) individually, we'd like to reduce API-requests by having an endpoint to get all permissions that are given to an objects at once.

This is why I've added the endpoint /objects/<object_id>/permissions/.
On GET the endpoint will return a dictionary of all "permission-types":

{
    "users": {
        1: "grant"
    },
    "basic_groups": {
        2: "read"
    },
    "projects": {
        2: "read"
    },
    "authenticated" : "none",
    "anonymous": "none"
}

Using PUT on the same endpoint with a dictionary of at least one key from above will perform the given permissions to the object. (This does not remove all existing permissions of the given object and set the new ones. It will only change or add the given permissions)

We also like to add the endpoint /objects/permissions/copy/. This endpoint uses the already implemented function "object_permissions.copy_permissions()" and is able to receive multiple "source"-"target" object pairs, where the target's permissions get overwritten by the source object's permissions.
Performing a POST-request with json-data like:

[
    {
        "source": 1,
        "target": 2
    },
    {
        "source": 4,
        "target": 3
    }
]

... will for example overwrite the object-permissions from object {2} with those from object {1}.

You can either perform the request with a list of json-objects or just with one json-object like:

{
    "source": 1,
    "target": 2
}

[FlorianRhiem]

Hey Florian, thank you for the pull request!

I've made some small improvements to help our static analyzers pass and to fix one of our tests. I've also moved the function for mapping object permissions to a dict that can be used for JSON to the actual object permissions api module, rather than the utils module for the various APIs.

Currently, object permissions are already part of the API, so it'd be best to stay consistent with the existing endpoints and naming. I know basic_groups matches the frontend better than just groups, which is the name they were previously known as in the frontend and are still known as in the rest of the code, but consistency here is better than improving it in some parts. Similarly, permissions for anonymous users are currently exposed via /api/v1/objects/(int: object_id)/permissions/anonymous_users and those for authenticated users are exposed via /api/v1/objects/(int: object_id)/permissions/authenticated_users, so the new endpoint should match those names rather than deviating from them.

For the permissions copying, having an _id suffix, or even _object_id suffix might help make target and source clearer.

Also, the documentation could still use some improvement, e.g. the PUT on /api/v1/objects/(int: object_id)/permissions/ doesn't state what it does, it only gives two clarifications, and the JSON inputs are undocumented for both that and the copy endpoint.

The API endpoints could use some more defensive coding, e.g. you assume that request arguments are the right type, rather than making sure, so rather than giving users a Bad Request error and information on what's wrong, the code will result in an Internal Server Error instead.

Finally, if you're okay with that, please add yourself to the list of authors in pyproject.toml :)

Let me know if any of that isn't clear enough or you need help!

[flooobauer]

Hey Florian, thank you for the tips.

I've made the adjustments you've mentioned.

The naming for the API should now be consistent and for the copy endpoint, I've added the suffix _object_id.

I tried to improve the type checking of the requests arguments. Let me know if there are still situations where the code will result in Internal Server Errors.

If there's anything missing or still needing improvements, I am open to have a look at it.

[timhallmann]

I missed it before, but /api/v1/objects/<int:object_id>/permissions/ isn't atomic. Validation and Execution are interspersed. Could you please validate all changes prior to executing anything, same as you did in /api/v1/objects/permissions/copy/?

And maybe include these new endpoints in docs/changelog.rst. If you do, please also add "Added OIDC session expiration and Back-Channel Logout". :)

[FlorianRhiem]

mypy complains about this line: sampledb/api/server/object_permissions.py:207: error: Incompatible types in assignment (expression has type "int", variable has type "Literal[False]") [assignment] because both source_object_id and target_object_id contains the result of the comparison with None, so both are False when everything goes well, which will be converted to 0 with int, which obviously wasn't your intention. assign source_object_id to item.get("source_object_id") first and then do the comparison, the same for target_object_id and you'll avoid that issue.

[FlorianRhiem]

Best check that type(source_object_id) is int (and same for target_object_id), instead of attempting a conversion for a copy of them, as this way you'll end up passing strings to copy_permissions. Also fix the error message for target_object_id using source_object_id in its text

[FlorianRhiem]

Re-think this loop ;)

[FlorianRhiem]

Currently, multiple copy requests at once is broken. This test doesn't find it, because it copies the permissions back and forth, so there's no way to validate that it worked.

[flooobauer]

Hey Florian,
I think I've addressed the issues we were talking about. Let me know if I missed something.

[FlorianRhiem]

It looks mostly good, but there is still some unvalidated user input for user, group and project IDs that will result in 500 errors. Also, best use type(x) is int rather than isintance(x, int) to avoid having to make sure it's not a bool, for example. Same for the object_id check in the copy endpoint.

Once that's done, could you squash the commits? :)

Thanks!

[flooobauer]

It looks mostly good, but there is still some unvalidated user input for user, group and project IDs that will result in 500 errors.

I dont see where a potential 500 could be thrown. For which endpoint under which conditions do those get thrown?

[flooobauer]

Also, best use type(x) is int rather than isintance(x, int) to avoid having to make sure it's not a bool, for example. Same for the object_id check in the copy endpoint.

Sure, I'll change it. Maybe this should be addressed for all other endpoints in the future to ensure consistency within the code.

[FlorianRhiem]

It looks mostly good, but there is still some unvalidated user input for user, group and project IDs that will result in 500 errors.

I dont see where a potential 500 could be thrown. For which endpoint under which conditions do those get thrown?

If you pass user, group or project IDs to api/v1/objects/<object_id>/permissions that aren't actually integers, these are passed straight into functions like check_user_exists which pass them into SQLAlchemy (and psycopg2), which then raises errors such as this:

  File "/home/sampledb/venv/lib/python3.14/site-packages/sqlalchemy/engine/base.py", line 1967, in _exec_single_context
    self.dialect.do_execute(
    ~~~~~~~~~~~~~~~~~~~~~~~^
        cursor, str_statement, effective_parameters, context
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/home/sampledb/venv/lib/python3.14/site-packages/sqlalchemy/engine/default.py", line 952, in do_execute
    cursor.execute(statement, parameters)
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type integer: "1.0"
LINE 3: WHERE groups.id = '1.0'

Aside from 1.0, things like null or "" also cause the same issue. The IDs should be converted to integers before passing them to function such as check_user_exists which expect integers.

Sure, I'll change it. Maybe this should be addressed for all other endpoints in the future to ensure consistency within the code.

Yeah, the API needs some work, one of my colleagues is currently refactoring the input validation, for example. Sorry for the confusion!

[flooobauer]

If you pass user, group or project IDs to api/v1/objects/<object_id>/permissions that aren't actually integers, these are passed straight into functions like check_user_exists which pass them into SQLAlchemy (and psycopg2), which then raises errors such as this:

I see, thank you. Fixed that now.

I tried to squash my commits but it seems like my last try to sync my branch with develop branch might broke something as I have those commits now in this PR. Do you have any suggestions to fix that?

[FlorianRhiem]

I just tried the following locally:

• check out your batch_permissions branch
• run git rebase -i upstream/develop (with upstream being whatever you've named the remote for this repo here)
• you should see a short-ish list of commits, with your squashed commit Add bulk permission management and copy functionality via API among them
• remove all the commits you don't want to include, so all commits that aren't supposed to be part of this PR. That should probably just be Add bulk permission management and copy functionality via API (a29d840)
• Save and, if everything works like it did for me, you should get a single commit that sits on top of the current develop branch