On Tue, 9 May 2023 22:09:26 GMT, Alexander Zvegintsev <azveg...@openjdk.org>
wrote:
> > I want to work through the scenarios and how much of it is specific to the
> > behaviours of the API you are using and so forth. Since you use the
> > Preferences API for saving the token, if you keyed it off the Robot class
> > rather than the internal API class then anyone using the Preferences API
> > could delete the token, couldn't they ?
>
> Right now it can be read with a simple call:
> `System.out.println(Preferences.userRoot().node("/sun/awt/screencast").get("restoreToken",
> ""));` (modification is also simple).
>
> Moving to Robot doesn't change anything in this regard. So anyone can delete
> it.
I was thinking it better to make it an API class, but we can discuss later when
we are sure we want the API at all.
> > And we presumably can store alongside the token, info about the screens
> > attached at the time we obtained the token ?
>
> We can try to do that and switch it depending on the area requested, but I
> tried to keep it simple.
If it can avoid the need for dialogs in some additional cases, its worth it, but
we can add this later, correct ?
But it may be worth doing now - see comments below.
>
> > Can you point (here in the implementation review) to the exact native
> > functions that popup up the dialog ? I'm still trying to find my way
> > through that code. Is it opaque to the caller (us) what the user actually
> > approved ?
>
> (It is collapsed by the github in the review, so links are to the branch)
>
> basically if you [didn't provide a correct token or no token at
> all](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L530-L537)
> to [SelectSources
> call](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L539-L546)
> it will prompt with dialog right after [the Start
> call](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L677-L685)
>
> > I can't imagine why we would need to reset "we have all permissions" to "we
> > have no permissions" so I'm supposing the issue is when they didn't grant
> > permissions .. but if they "deny" permissions that's surely different than
> > "this token is no longer valid" ?
>
> It is not for the "we have all permissions" case, it is for the case when we
> only have a permission to some screens. So it won't hurt.
But how does the app know when to call it ?
>
> > The first time we need a permission for the screencast API there is a
> > dialog. The user makes some choices and we then choose to save a token so
> > that the user isn't prompted again. We must at least know though that they
> > approved SOMETHING but we don't know what ? Or do we ?
>
> No we don't.
>
> We receive the response
> [callbackScreenCastStart](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L586-L591)
> as a separate streams for each allowed display.
>
> Based on this response we [building a list of **available**
> displays](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L73).
> So denied screens will not be there.
OK, so to make sure I understand, you use the token to see if you can obtain a
stream for all connected displays. If any "fail" you know the user didn't
approve that.
So we can do this every time right ?
And in conjunction with knowing which screen[s] were connected when we captured
the token we know the situation changed and so we know it is appropriate to
ignore the stored token - causing the user to be re-prompted.
And even if the screens have changed if we already have the token we need for
THIS screen capture we know whether we already have the permission we need. And
if we do then we just use it and don't cause a user re-prompt.
At this point I think we've done everything that "revoke" was needed for.
Then [if I have everything right] the only open question is whether when
permissions to the required screen or screens was previously denied,
do we re-prompt ? My take is that we do not "store" a token that denies access.
So next time the app is run they will be re-prompted.
But we *probably* don't want to keep re-prompting within this running instance
of the JDK.
BTW "the app" means "any app that can find this token, doesn't it" ?
This is probably what we want, and this is also how the Apple permissions work
..
but maybe non-intuitive to users who don't know that both App A and App B are
written in Java.
I don't propose doing anything here, since if we tied it to a specific "class"
then
it would not work very well for 500 individual regression tests that each need
robot :-)
>
> At this point we can compare this result with the screens got from X11
> API(won't be compatible with Wayland, where we also want to reuse it later)
> or with Wayland(+1 dependency at this point).
>
> > If they DENY, would we save that ? Interesting question because if we don't
> > save something we'll keep spamming them until they approve :-)
>
> The [DENY
> case](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L586-L591)
> propagated to java and we are throwing the security exception.
>
> > Next time, what happens if for some reaon the token is no longer valid ?
>
> It just prompts a permission request again.
>
> > And what might make it invalid ?
>
> Adding or removing a display for example.
>
> > If the user denied screen 2, or added screen 2 later, would that not result
> > in an invalid token and we'd know to ask again ? No reset needed ? Also I
> > can't figure out if "validating" the token can cause the dialog as a side
> > effect or if there's some API we explicitly call to request a new token.
>
> Let me elaborate how this `restore_token` works:
>
> We may or may not provide the `restore_token` while calling `SelectSources`.
>
> If the token provided is not valid (we have no control over this, it's
> checked on the system side), the permission prompt will appear.
>
> Later, after granting permissions, the `restore_token` is returned by
> [Start](https://flatpak.github.io/xdg-desktop-portal/#gdbus-method-org-freedesktop-portal-ScreenCast.Start)
> callback. We should
> [update](https://github.com/azvegint/jdk/blob/482471df61490120590bc49886f76dc99c3d7dd8/src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c#L593-L611)
> it, as it may not be the same as we provided before.
>
> When we are calling the `resetScreenCapturePermission` we are not actually
> revoking the token from the system, we are just doesn't add it to the
> `SelectSources` call. (but, if needed, we can provide it later and it should
> work).
I think most people reading the API would interpret it as "rm <the token>, not
"don't use the token". And since its not tied to a particular invocation of
Robot screen
capture APIs it means that if you then call System.exit(0) right afterwards, so
no new token is captured, it didn't do what you thought it did.
Also if we follow the suggested policy of not storing tokens which don't grant
permission
then we'd also not over-write it in that case.
>
> So whenever we don't provide a valid token, we get a confirmation prompt.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13803#issuecomment-1542866062
