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

Consumption of User Gesture #25

Open
Brandr0id opened this issue Apr 16, 2020 · 27 comments
Open

Consumption of User Gesture #25

Brandr0id opened this issue Apr 16, 2020 · 27 comments
Assignees
Labels
blocked Blocked on something future Will consider for a future revision

Comments

@Brandr0id
Copy link
Member

In 3.1.1. User Agent storage access policies

The user gesture appears to be consumed if a prompt is rejected but not during any implicit grant without a prompt or if a prompt is allowed.

  1. If user expression of permission is false, let w be doc’s Window object and run these steps:
    1. If w has transient activation and user expression of permission is false, consume user activation with w.

This seems to be the only case the gesture would be consumed by this API, is there a specific reason to only consume it here or can we be consistent and always pass it through?

@johnwilander
Copy link
Collaborator

In 3.1.1. User Agent storage access policies

The user gesture appears to be consumed if a prompt is rejected but not during any implicit grant without a prompt or if a prompt is allowed.

  1. If user expression of permission is false, let w be doc’s Window object and run these steps:

    1. If w has transient activation and user expression of permission is false, consume user activation with w.

This seems to be the only case the gesture would be consumed by this API, is there a specific reason to only consume it here or can we be consistent and always pass it through?

The specific reason is that if the user has explicitly tapped/clicked "Don't allow," the iframe should not be able to continue and call other APIs which require a user gesture, such as open a popup, without acquiring another gesture. We've already seen misuse of the API with infinite prompting for instance.

@Brandr0id
Copy link
Member Author

Thanks for clarifying.

The specific reason is that if the user has explicitly tapped/clicked "Don't allow," the iframe should not be able to continue and call other APIs which require a user gesture, such as open a popup

I don't know that I have a strong opinion on this either way but this seems slightly separate from the concern/abuse seen with infinite prompting from this API. Should callers not be allowed to have a fallback mechanism where if storage access is denied they open a popup or take another action where they may have access to storage and can provide an alternate experience?

In either case does it make sense to consume, or not consume, the gesture equally in the implicit and explicit deny scenarios?

We've already seen misuse of the API with infinite prompting for instance.

Agreed we don't need any prompt loops. Is this solved by the explicit deny scenario setting the flag on the document to prevent future API calls and immediately rejecting the promise?

@jackfrankland
Copy link

A third party can hang both the requestStorageAccess and the window.open from the same user action:

button.onclick = () => {
  const popup = window.open('./popup.html', '_blank');
  document.requestStorageAccess()
    .then(() => {
      popup.close(); // or integrate a nicer user journey to close the window
    })
    .catch(() => {
      // leave the popup open
    });
});

It's ugly, but it's viable and would be the only remaining option a third party has instead of requiring two user actions, and I think it's likely to be utilised. Perhaps it's better not to consume the user action, and prevent the prompt from reappearing. I thought there was a limit already in place in Safari?

@annevk
Copy link
Collaborator

annevk commented Apr 20, 2020

A popup also consumes so that's not a viable workaround (fortunately).

@jackfrankland
Copy link

@annevk are these methods supposed to consume the gesture synchronously then, rather than the consumption being "non-propagation" of user gesture in the async callback?

My testing of the current implementation shows that requestStorageAccess can resolve after window.open has been invoked.

@annevk
Copy link
Collaborator

annevk commented Apr 20, 2020

Yeah, I think that's how user activation is supposed to work (model is detailed in the HTML Standard, but not all APIs have explicitly adopted it yet).

@jackfrankland
Copy link

jackfrankland commented Apr 20, 2020

OK great. I wasn't aware of this, thanks.

At the risk of adding further noise... because the requestStorageAccess method itself will not consume the user activation (only the explicit reject will, from a non-blocking prompt), the following is still viable I believe, even if window.open were to consume it:

button.onclick = () => {
  let popup;
  document.requestStorageAccess()
    .then(() => {
      setTimeout(() => {
        popup.close(); // or integrate a nicer user journey to close the window
      }, 0);
    })
    .catch(() => {
      // leave the popup open
    });
  popup = window.open('./popup.html', '_blank');
};

@annevk
Copy link
Collaborator

annevk commented Apr 20, 2020

Wouldn't it be an implementation bug to not consume it call time? The whole idea is to prevent that kind of abuse.

@jackfrankland
Copy link

My understanding of the proposed spec is that the calling of requestStorageAccess will not consume the user gesture, to allow for user gesture dependent APIs to be called when the user grants access, or, apparently, access is denied implicitly.

@johnwilander
Copy link
Collaborator

@jackfrankland While there might be workarounds sites can pull off, the existence of those are not arguments for making the spec allow it. The intention of the behavior after an explicit rejection by the user is to not allow further API calls which require a user gesture unless the user again interacts with the iframe.

The reason why I brought up infinite re-prompting is that it has already happened and so we know we need a way for users to say “No” and move on to other things they want to do.

@annevk
Copy link
Collaborator

annevk commented Apr 20, 2020

Why consume only at rejection time though? Why not consume immediately when the API is invoked?

@Brandr0id
Copy link
Member Author

Why consume only at rejection time though? Why not consume immediately when the API is invoked?

I seem to recall this was previously the case in current implementations but there was a use-case that was broken in either Firefox or Safari that prompted for the gesture to no longer be consumed in the success case. @johnwilander or @ehsan may have more context on this.

My understanding is that consuming the gesture in these APIs is mostly about preventing abuse. We wouldn't want to continually open popups like we wouldn't want to continue to prompt for access in a loop. Is there a concern about chaining APIs though? It seems there are two issues we probably shouldn't conflate:

  1. Should chaining APIs that requires user gesture be allowed?
  2. How can we prevent abuse with subsequent SAA calls (e.g. in rejection scenario)?

I think for 1. not consuming the gesture in the allow scenario or when the API is first called opens the door to this being okay. For 2. we already set a flag on the Document in the prompt rejection scenario. I believe this may be enough by itself to prevent abuse of the prompt/API since subsequent calls during this or future gestures will be rejected for the lifetime of the current Document.

@othermaciej
Copy link

Some embedded widgets want to pop up UI in a separate window to let the user log in, after requesting storage access and discovering that the user isn't logged in to the service. They don't want to make the user click the widget a second time in that case, which seems reasonable. This is only relevant after an acceptance. I think that's the use case we want to preserve.

Not sure if this is best done by consuming user gesture on reject only, or consuming it on call and then creating a new fake user gesture only when the promise is fulfilled without rejection.

@annevk
Copy link
Collaborator

annevk commented Apr 21, 2020

I wonder if @mustaqahmed could chime in here. It does seem lot cleaner to me that you consume directly and then unconsume/fake a gesture in the task that fulfills the promise allowing another bit of UI.

@hober
Copy link
Member

hober commented Apr 21, 2020

@annevk, Gecko currently propagates the user gesture to the resolve handler; see dom/base/Document.cpp#l15620.

@mustaqahmed
Copy link

If we are consuming here only to prevent "permission spamming", shouldn't it be handled through some intervention on Permissions API? FYI, there is an ongoing discussion to let Permissions "handle" the user activation dependency for APIs like these.


Re fake user activation: Unfortunately we don't see it as a possible solution here. The user activation spec allows activation triggering only through real user interaction. The idea of faking (or the ability to un-consume) seems to clobber the triggering mechanism. For example, it is not clear how a fake-trigger would work in a frame tree after the consumption operation had reset the state across the frame tree. Then there is the possibility that a malicious site may exploit a slow-promise-fulfillment to extend the expiry time of the original user activation. (I think we also have to be careful with multiple async APIs trying to fake in parallel, but I couldn't come up with an example.)

There is a general sentiment in the spec community that the browser should represent "the user" when it comes to user activation state, so letting developers control the state sounds terrible. We (Chrome) agreed to this point after long discussions on attempted delegation mechanisms (e.g. here).

@hober hober self-assigned this Apr 28, 2020
@hober hober added the agenda+F2F Request to add this issue or PR to the agenda for our upcoming F2F. label Apr 28, 2020
@hober
Copy link
Member

hober commented Apr 29, 2020

If we are consuming here only to prevent "permission spamming", shouldn't it be handled through some intervention on Permissions API?

Maybe. We're tracking possible integration with the Permissions API in #32.

@annevk
Copy link
Collaborator

annevk commented Apr 30, 2020

@mustaqahmed I'm not entirely convinced those concerns apply. I don't think explicit propagation is needed/expensive as all these origins share the window agent. They would have synchronous access to any such state. And in this case the site ends up with UI and can then on success do something else. If instead we don't consume (or only consume upon rejection), there's much more potential for abuse between showing the UI and ending up with a rejection.

@mustaqahmed
Copy link

I see, the consumption idea here is unlike the regular consumption. Perhaps it is like "pausing" the user activation for the current origin---did I get your thoughts right?

@annevk
Copy link
Collaborator

annevk commented May 5, 2020

Yeah, paused-pending-a-decision would be another way of describing this.

@hober hober removed the agenda+F2F Request to add this issue or PR to the agenda for our upcoming F2F. label Jun 8, 2020
@hober hober added agenda+ Request to add this issue to the agenda of our next telcon or F2F interop Tracking differences in existing implementations and removed agenda+ Request to add this issue to the agenda of our next telcon or F2F labels Jul 6, 2020
@johannhof
Copy link
Member

So to me it feels like this pausing mechanism/user activation continuation should be specified more closely before we can consider using it here. If I'm reading the comments correctly then Safari isn't particularly concerned about the corner cases and I would agree that there's no point in spending a lot of effort in perfecting this protection when there are other easy ways to obtain and abuse user activation, for a determined attacker. The simple consumption on failure is definitely worth doing and this is tracked in https://bugzilla.mozilla.org/show_bug.cgi?id=1557097 on Firefox side, which I hope we can bump the priority on.

@johannhof johannhof removed the interop Tracking differences in existing implementations label Dec 16, 2020
@johannhof
Copy link
Member

Which is to say I'm not sure this needs the "interop" label because Firefox thinks this is valid behavior and we just haven't gotten around to it yet :)

@johannhof
Copy link
Member

...though I realize that Edge doesn't seem to align on this either. Let's have the label.

@johannhof johannhof added the interop Tracking differences in existing implementations label Dec 16, 2020
@annevk
Copy link
Collaborator

annevk commented Dec 17, 2020

It seems continuation would be a matter of invoking "activation notification" at the appropriate time and that's it. It's probably worth bringing such usage to the attention of whatwg/html though.

@johannhof
Copy link
Member

In our ad-hoc meeting we decided that we want to leave consumption on explicit rejection in the spec for now, pending a more thorough discussion of a "pausing"/reactivation mode for user activation that should happen at whatwg/html.

@annevk would you have time to file an issue for that? :)

@johannhof johannhof removed the interop Tracking differences in existing implementations label Jan 28, 2021
@johannhof
Copy link
Member

This issue recently came up again on Firefox side as we're trying to address some open bugs in our SAA implementation. I filed whatwg/html#7192 for getting some clarity on the "pause" model, but did not CC everyone in this issue so please subscribe as needed :)

@hober hober added the blocked Blocked on something label Feb 8, 2022
@hober
Copy link
Member

hober commented Feb 8, 2022

From the editors' call today: This is blocked on whatwg/html#7192; we'll be able to resolve on this side once that gets resolved on the HTML side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked on something future Will consider for a future revision
Projects
None yet
Development

No branches or pull requests

8 participants