camera: fix permission check in OpenPipeWireRemote

[mcatanzaro]

6cd99b0 changed the logic that the camera portal uses to look up permissions for the AccessCamera method. Applications first call AccessCamera to ensure they have camera permission and to prompt the user if permission is missing, then they call OpenPipeWireRemote, which fails if permission is missing. The permission lookup logic needs to be the same in both places. Currently when running Snapshot launched by GNOME Shell (rather than launched in a terminal), Snapshot passes AccessCamera's permission check, then fails OpenPipeWireRemote's permission check, causing camera access to be denied without allowing the user to grant permission.

Also, since the same commit the code uses the XdpAppInfo on a secondary thread. I suspect this is unsafe, and the original code avoided doing so; therefore, let's be careful and move this logic to the main thread so that the secondary thread only receives a copy of the app ID, as before.

https://gitlab.gnome.org/GNOME/snapshot/-/issues/267

[mcatanzaro]

@grulja mentioned in the Snapshot issue:

I see other portals accessing request->app_info from the secondary thread. I think your patch is correct and don't see any potential problem with it. The only thing that is probably missing is moving REQUEST_AUTOLOCK (request) up before calling query_permission_sync() in handle_access_camera_in_thread_func().

Hi maintainers, do we really need the g_object_set_data()/g_object_get_data()? It has been there since the camera portal was first introduced by @jadahl.

Then I guess the lock is just in the wrong place and should also be fixed. I can do that in this pull request if you prefer.

[grulja]

@grulja mentioned in the Snapshot issue:

I see other portals accessing request->app_info from the secondary thread. I think your patch is correct and don't see any potential problem with it. The only thing that is probably missing is moving REQUEST_AUTOLOCK (request) up before calling query_permission_sync() in handle_access_camera_in_thread_func().

Hi maintainers, do we really need the g_object_set_data()/g_object_get_data()? It has been there since the camera portal was first introduced by @jadahl.

Then I guess the lock is just in the wrong place and should also be fixed. I can do that in this pull request if you prefer.

I don't know why it has been done like that and I'm certainly not a glib expect so most of the API is a stranger to me, but this

xdg-desktop-portal/src/account.c

Line 103 in 7928c83

ruri = xdp_register_document (image, xdp_app_info_get_id (request->app_info), XDP_DOCUMENT_FLAG_NONE, &error);

looks like a similar case.

Anyway, it looks good to me, with the suggestion to hold the lock around request before calling query_permission_sync().

[mcatanzaro]

Discussed with Georges on Matrix:

• I need to move the locking, like you suggest
• I will avoid using XdpAppInfo on the secondary thread; we suggest filing a bug report for the case in account.c

[mcatanzaro]

I need to move the locking, like you suggest

This broke test-camera, so I'll remove this change.