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

Update the version detail API to report host_permissions #9105

Closed
bobsilverberg opened this issue Mar 1, 2023 · 8 comments · Fixed by mozilla/addons-server#20436
Closed
Assignees
Labels
component:api migration:2024 priority:p3 This priority level reflects our backlog. repository:addons-server Issue relating to addons-server state:verified_fixed
Milestone

Comments

@bobsilverberg
Copy link
Contributor

bobsilverberg commented Mar 1, 2023

This is a follow-up to #9104. Once we are storing host_permissions for MV3 add-ons, we should update the API to return those stored host_permissions.

┆Issue is synchronized with this Jira Task

@bobsilverberg bobsilverberg self-assigned this Mar 1, 2023
@bobsilverberg bobsilverberg added this to the 2023.03.09 milestone Mar 1, 2023
@bobsilverberg bobsilverberg added component:api priority:p3 This priority level reflects our backlog. labels Mar 1, 2023
@eviljeff
Copy link
Member

eviljeff commented Mar 2, 2023

I think we should hold off on this until we know what the actual need/request is. What frontend is going to display, and how - if it's just that the urls aren't omitted from the permissions block for MV3 add-ons the serializer should just return both urls and permission names in the existing permissions field.

@bobsilverberg
Copy link
Contributor Author

With the filing of #14670, it looks like we're going to need this. Based on the lengthy discussion at #9104, I feel like the following are our best options:

  1. Add a property to the API response for host_permissions, returning exactly what we find in host_permissions in the manifest.
  2. Take the contents of host_permissions from the manifest and add them to optional_permissions in the API response. This is based on this comment that says that host_permissions are optional permissions.

As discussed in the other issue, I am in favour of 1. It does involve some extra processing on the client, but it also means we aren't coupling the API response to our specific UX requirements at this time. My concern with option 2 is that, if we decide to do something different with host_permissions on the client in the future, this will necessitate an API change.

@diox @eviljeff @kplotnik What do you think?

@willdurand
Copy link
Member

Take the contents of host_permissions from the manifest and add them to optional_permissions in the API response. This is based on this comment that says that host_permissions are optional permissions.

This is going to mix API and host permissions, which MV3 solved with the introduction of host_permissions. Also, as I mentioned, there might be some optional_host_permissions key in the future.

@eviljeff
Copy link
Member

eviljeff commented Mar 6, 2023

Note the MV2 add-ons (the vast bulk of extensions on AMO today) mix the host and API permissions, and frontend processes them to display them separately* (in the same card). So we should consider how MV2 permissions are handled too.

*option 1 would, as far as a I understand it, lead to less processing on the client, because host and API permissions wouldn't need to be split out from a single list of optional permissions(?)

@willdurand
Copy link
Member

Note the MV2 add-ons (the vast bulk of extensions on AMO today) mix the host and API permissions

Yes, but they are also all required so that makes sense.

and frontend processes them to display them separately* (in the same card). So we should consider how MV2 permissions are handled too.

Are they displayed separately? My understanding is that we have two sections, and for each, we have both API and host permissions. In the screenshot below, we only have host permssions (some required, some optional):

Screenshot 2023-03-06 at 18 49 16

And here we have both API and host permissions, all required:

222222273-7bbbf300-6688-4ef8-8002-90e8965d5ead(1)

@eviljeff
Copy link
Member

eviljeff commented Mar 6, 2023

Are they displayed separately?

Separately on the same card. From #9104

We are already reporting regular and host permissions in the same card on the front-end, although we do process them so host permissions will show up last.

@ioanarusiczki
Copy link

@bobsilverberg

I tried with MV2 and MV3 -> the detail/version endpoints return permissions , optional permissions and host permissions

host permission

@KevinMind
Copy link
Contributor

@KevinMind KevinMind transferred this issue from mozilla/addons-server May 4, 2024
@KevinMind KevinMind added repository:addons-server Issue relating to addons-server migration:2024 labels May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api migration:2024 priority:p3 This priority level reflects our backlog. repository:addons-server Issue relating to addons-server state:verified_fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants