fix(api): use requestor's permissions for request approval

[kinggeorges12]

… than the logged in user account

In the POST /request API call, the permissions make the request and quotas are all inherited from the "requesting user". The only difference comes from the auto-approve permissions, which for some reason use the admin or API key holder permissions.

To make this code more consistent with the expected behavior, I updated the code to look at the requestUser permissions for auto-requests.

Description

The users reported a bug on my plugin that uses Jellyseerr: kinggeorges12/JellyBridge#17

I noticed that the behavior in this MediaRequest class was inconsistent, where the user requesting an item would be checked for quotas and requests (like do they have the ability to add requests AT ALL), but when checking for user overrides or auto-approve, the API keyholder or authorized user permissions were checked.

Bug: when requesting media through the API for another user, the approval process is skipped for the requesting user (userId in the API call for POST /requests). When creating new requests as the root account, the request is auto-approved since this permission cannot be changed for admin accounts.

What this means is that there is no way to make the approval process work when sending requests as another user from an admin account. Although this is noted in the API "If the user has the ADMIN or AUTO_APPROVE permissions, their request will be auomatically approved.", the comment is completely misleading, because the "user" it is talking about is NOT the userId provided by the function. I think this change brings it more in-line with expected behavior of this API endpoint.

I already tested this change here for v2.7.3: https://hub.docker.com/repository/docker/kinggeorges12/jellyseerr/tags/2.7.3-jb/

Screenshot (if UI-related)

To-Dos

• Disclosed any use of AI (see our policy)
• Successful build pnpm build
• Translation keys pnpm i18n:extract
• Database migration (if required)

Issues Fixed or Closed

• Fixes #XXXX

[gauthier-th]

Did you test this? I'm pretty sure using user is the correct way of doing this, especially for the override rules. We want to check that the user that made the request has the ADVANCED_REQUEST permission, not the user to whom the request was assigned. And I believe it's the same for the other changes.

There are 2 different things here:

• user: the person who made the request
• requestUser: the user to whom the request was assigned. By default it's the same as user, but can be changed by the user if he has MANAGE_REQUEST permission

[kinggeorges12]

Hi @gauthier-th , thanks for checking this PR. To answer your question: yes, I did test this using the docker image that I mentioned earlier, and tests pass for ensuring permissions of the logged in (or API caller) user to manage requests. Given two test accounts "usertest" and "root":

• I try to request a movie with root logged in providing the userId=2 ("usertest"), and the movie is sent to the queue for requests to accept/decline.
• If I try to request a movie with "usertest" logged in without specifying the userId, the request is also sent to the queue.
• If I try to request a movie with "usertest" logged in and specifying any userId, the error message appears { "message": "You do not have permission to modify the request user." }

I could not find any references to the "ADVANCED_REQUEST" permission in the code, so it may be an artifact of when this code was originally written. The comment on line 204 might now be attributed to the "MANAGE_REQUESTS" permission, which is required to actually submit the request.

The code flow is a bit confusing, because requestUser is initially set to user. However, on line 60 you can see this code flow that basically checks if the user is trying to create this request on behalf of another user:

    let requestUser = user;

    if (
      requestBody.userId &&
      !requestUser.hasPermission([
        Permission.MANAGE_USERS,
        Permission.MANAGE_REQUESTS,
      ])
    ) {
      throw new RequestPermissionError(
        'You do not have permission to modify the request user.'
      );
    } else if (requestBody.userId) {
      requestUser = await userRepository.findOneOrFail({
        where: { id: requestBody.userId },
      });
    }

From what I am reading, if the user is trying to request on behalf of another user, the userId would be set in the API call. If the current logged in user (which is referred to as requestUser because of the line let requestUser = user) is missing the MANAGE_REQUESTS or MANAGE_USERS permissions, then an error is thrown. Then at line 68, having passed the permission check for the API caller, the userId provided in the API call is set to the requestUser.

The only change I would make to my submission is to make the logic flow more clear on these lines I quoted above. The current state makes it ambiguous as to whose permissions are actually getting checked, since the requestUser changes from the API caller to the userId in the API call. In fact, I would say this if statement is TOO stringent, and should only check the MANAGE_USERS permission if the userId provided is different than the current logged in user, since the REQUEST_TV or REQUEST_MOVIE permissions for the request user is already checked in the code following this user permission check (lines 78-110).

Also, in debugging this i noticed an error in the code for existing requests. When checking for is4k in the request body on line 165, the where clause does not specify a default is4k value. This allows users to send multiple requests for non-4k content without hitting the error { "message": "Request for this media already exists." }.

      .where('request.is4k = :is4k', { is4k: requestBody.is4k })

For this other bug, I can create an additional pull request if needed.

[kinggeorges12]

Did you test this? I'm pretty sure using user is the correct way of doing this, especially for the override rules.

I had a question about this part of your comment, as I have not used override rules. The override rules seem to be for certain rules of that request user to change these request attributes: rootFolder, profileId, tags. When requesting content on behalf of another user, wouldn't you want to use the override rules for the target user of the request, not the override rules for the API caller?

[fallenbagel]

I'm kinda confused what you're trying to accomplish here. What was the issue? (Apologies it's still not very clear).

If your plugin connects to seerr using api-keys, it is expected that the request is handled as a superuser request being requested on behalf of someone. That is the expected behaviour. For the request to be handled as a certain user, it needs to be cookie-auth based. This is how our web client works. This is how other third-party streaming apps that has seerr support also works.

[fallenbagel]

Did you test this? I'm pretty sure using user is the correct way of doing this, especially for the override rules.

I had a question about this part of your comment, as I have not used override rules. The override rules seem to be for certain rules of that request user to change these request attributes: rootFolder, profileId, tags. When requesting content on behalf of another user, wouldn't you want to use the override rules for the target user of the request, not the override rules for the API caller? Maybe I'm missing something because there is no reference to override rules in the 2.7.3 interface, so I am not exactly sure how they are implemented.

Override rules are currently not supported for users with advanced_request permission. Requesting on behalf of someone is still a request from an admin user and that user has advanced_request permission and their permissions will get applied. This is intended behaviour as these users permissions should always be superior even if they're requestingon behalf of someone else. There is an open PR open to support override rules for advanced_request users.

Essentially what this PR does will break the current intended behaviour completely.

[kinggeorges12]

I'm kinda confused what you're trying to accomplish here. What was the issue? (Apologies it's still not very clear).

I agree, the bug is not exactly clear, because new requests on behalf of a user should be "superuser request being requested on behalf of someone" like you said. I am trying to explain that the implementation of on behalf of someone is inconsistently applied. I contend that when making a request on behalf of someone, ALL rules of that request should be applied based on the on behalf of user permissions.

Current state:

• quotas are applied as on behalf of user
• permissions to request (including 4k flag) are applied as on behalf of user
• request workflow to approve/deny is applied as the superuser

Bug: Users expect that when requesting content on behalf of someone, that the request still goes through the status approval workflow.

Fix: Add the same permissions for the on behalf of user to the PENDING status if that on behalf of user does not have AUTO_APPROVE permissions for that media type.

[kinggeorges12]

Override rules are currently not supported for users with advanced_request permission. Requesting on behalf of someone is still a request from an admin user and that user has advanced_request permission and their permissions will get applied. This is intended behaviour as these users permissions should always be superior even if they're requestingon behalf of someone else. There is an open PR open to support override rules for advanced_request users.

Essentially what this PR does will break the current intended behaviour completely.

Well it is certainly not my intention to break the intended behaviour. Could you explain why certain rules are applied to the on behalf of user versus the admin user?

I am not trying to be difficult, it just seems inconsistent to apply half of the rules for a request to the on behalf of user (can request and quota limit permissions) and the other half to the admin user (auto-approval of requests).

[kinggeorges12]

Fix: Add the same permissions for the on behalf of user to the PENDING status if that on behalf of user does not have AUTO_APPROVE permissions for that media type.

Sorry, I forgot to say that the override rules are also included in this fix. That is not completely necessary for my purposes, but it seems more consistent that override rules follow the rules for on behalf of user. It is not necessary for the bug that I am experiencing, but it does seem more consistent with expected behavior.

[Jototo39]

I'm kinda confused what you're trying to accomplish here. What was the issue? (Apologies it's still not very clear).

If your plugin connects to seerr using api-keys, it is expected that the request is handled as a superuser request being requested on behalf of someone. That is the expected behaviour. For the request to be handled as a certain user, it needs to be cookie-auth based. This is how our web client works. This is how other third-party streaming apps that has seerr support also works.

Hello,
I initially reported the issue in @kinggeorges12 plugin.
His Jellyfin plugin allows to populate a "Discover" library with chosen SVoD networks.
Then when the user set any item as "Favorite", it triggers a Jellyseerr request on behalf of that same user.

As a picture is worth a thousand words, here is the unexepected behavior I faced during testing:

• The top one went through the Jellyseerr standard process (requested by user in Jellyseerr, pending approval of admin)
• The bottom one was created by setting an item as favorite in Jellyfin, then was somehow auto-approved

My regular (non-admin) users does not have auto-approve permission.
Only my user (admin) has auto-approve, and can approve other users' requests.

I don't intend to tell if it's caused by an error in the Jellyseerr API or if it is the plugin implementation of the latter, I just wanted to further clarify the unexpected behavior I faced.

Have a great day,

[fallenbagel]

I'm kinda confused what you're trying to accomplish here. What was the issue? (Apologies it's still not very clear).

If your plugin connects to seerr using api-keys, it is expected that the request is handled as a superuser request being requested on behalf of someone. That is the expected behaviour. For the request to be handled as a certain user, it needs to be cookie-auth based. This is how our web client works. This is how other third-party streaming apps that has seerr support also works.

Hello,
I initially reported the issue in @kinggeorges12 plugin.
His Jellyfin plugin allows to populate a "Discover" library with chosen SVoD networks.
Then when the user set any item as "Favorite", it triggers a Jellyseerr request on behalf of that same user.

As a picture is worth a thousand words, here is the unexepected behavior I faced during testing:

• The top one went through the Jellyseerr standard process (requested by user in Jellyseerr, pending approval of admin)
• The bottom one was created by setting an item as favorite in Jellyfin, then was somehow auto-approved

My regular (non-admin) users does not have auto-approve permission.
Only my user (admin) has auto-approve, and can approve other users' requests.

I don't intend to tell if it's caused by an error in the Jellyseerr API or if it is the plugin implementation of the latter, I just wanted to further clarify the unexpected behavior I faced.

Have a great day,

That still doesn't answer the initial question i had (just asking for clarification purposes). If the plugin connects to jellyseerr via api keys, this is intended behaviour when userid is passed. If the plugin uses cookie auth, then it will request as the cookie auth user and useid will not be used. This is how its handled on every third party app that connects to jellyseerr including our very own web client as well. Its intended for api keys to act as a superuser

You can confirm this behavior via curl or even /api-docs. If you use apikeys and pass in a userid and request it will request on behalf of the user as an admin. If you use cookie auth, you wont need to pass in userId and will request as whoever is logged in.

[kinggeorges12]

That still doesn't answer the initial question i had (just asking for clarification purposes). If the plugin connects to jellyseerr via api keys, this is intended behaviour when userid is passed. If the plugin uses cookie auth, then it will request as the cookie auth user and useid will not be used. This is how its handled on every third party app that connects to jellyseerr including our very own web client as well. Its intended for api keys to act as a superuser

If the API key acted as a superuser consistently, then I would agree. However, the API key uses both the quotas and CREATE_REQUEST permission for the userId provided. This means that the superuser impersonates some of the characteristics of the userId but not all.

You can confirm this behavior via curl or even /api-docs. If you use apikeys and pass in a userid and request it will request on behalf of the user as an admin. If you use cookie auth, you wont need to pass in userId and will request as whoever is logged in.

The issue for me is that there is no way to impersonate a user at the superuser level. This is a common feature in major applications, e.g., Github, Linux, Hadoop. The use case I want is for a superuser to create a request exactly as a specified userId would create it, including all workflows associated with that on behalf of user, like the approval process. In the current API, there is no way to impersonate a user as a superuser, without knowing that user's cookie or API key.

[kinggeorges12]

This is how its handled on every third party app that connects to jellyseerr including our very own web client as well. Its intended for api keys to act as a superuser

Since I am building a 3rd party app connecting to Jellyseerr, I understand that there are others that function differently. However, a way for superusers to impersonate creating a request on behalf of a user would be extremely helpful in building my app.

I do not want to argue with you over intentions of your API, and I understand you have other implementations that might not be compatible with my approach. Could you tell me a better approach that I can use to achieve the same goals?

[gauthier-th]

Could you tell me a better approach that I can use to achieve the same goals?

For now, the only way of achieving this is through the cookie authentification (done by the user), or like you are doing right now, by making the request as an admin with the API key.

[gauthier-th]

I'm closing this as we consider the current behavior to be correct.

[kinggeorges12]

Could you tell me a better approach that I can use to achieve the same goals?

For now, the only way of achieving this is through the cookie authentification (done by the user), or like you are doing right now, by making the request as an admin with the API key.

Hi @gauthier-th , I just explained that the action I wanted to take was to impersonate a user as the admin. The method that you are suggesting does not do what I would like. What I was asking, is for a way to do the impersonation, not how to login as another user and submit a request.

Here is another example of how I could implement this: POST /user/request
This way, the user would have the request posted, and maintain the current functionality of the POST /request endpoint.

[gauthier-th]

I just explained that the action I wanted to take was to impersonate a user as the admin. The method that you are suggesting does not do what I would like. What I was asking, is for a way to do the impersonation, not how to login as another user and submit a request.

I understand that. I told you the closest methods you can use to do what you want. I know it's not exactly what you want, but there is no other way of doing it, and your PR does not fix it but is instead modifying a behavior we consider correct.

[kinggeorges12]

I just explained that the action I wanted to take was to impersonate a user as the admin. The method that you are suggesting does not do what I would like. What I was asking, is for a way to do the impersonation, not how to login as another user and submit a request.

I understand that. I told you the closest methods you can use to do what you want. I know it's not exactly what you want, but there is no other way of doing it, and your PR does not fix it but is instead modifying a behavior we consider correct.

OK the closest way still does not accomplish what I need for the API.

Would it be OK to create a new PR for a new endpoint that did exactly what I am asking? POST /user/{id}/request

[gauthier-th]

Would it be OK to create a new PR for a new endpoint that did exactly what I am asking? POST /user/{id}/request

I think it would be ok.

[kinggeorges12]

Would it be OK to create a new PR for a new endpoint that did exactly what I am asking? POST /user/{id}/request

I think it would be ok.

Thank you, @gauthier-th . I will work on another pull request to introduce that endpoint.