RFC: Make sever-side modality less strict by default

Flow
1

We consider changing server-side modality enforcement to not be enabled by default while still being available as an opt-in feature.

Background

We have this “gift that keeps on giving” as one Vaadin user described it: server-side modality. The idea is quite simple – a component that is inaccessible through the regular UI should not accept input from the user even if the user removes limitations through the browser inspector or sends a hand-crafted HTTP request. If a button is “out of reach” for the user, then there should be no way for the user to trigger the click listener of that button on the server. This is an essential security feature.

This has always been the case with Vaadin for components that are set as hidden or disabled from the server. Starting from Vaadin 23.0.0 in 2021, this same concept has also been applied for components that are behind a modal dialog. The problem with server-side modality is that it’s not reasonable for components to adjust their own behavior based on whether some completely different part of the component tree contains a modal dialog. Furthermore, many of the events that the components send are completely harmless from a security perspective e.g. confirming that a file upload has finished or that the client-side Grid no longer references items that have scrolled out of view.

This security feature has caused significant problems for users whereas the security threat that it protects against is relatively minor. From the perspective of the CIA triad, it can be seen as giving up lots of Availability for a minimal gain in Integrity.

Proposal

  • Add a new void setModalityMode(ModalityMode mode) method and a corresponding getter to Dialog, ConfirmDialog and LoginOverlay.
    • ModalityMode is an enum with the values MODELESS, VISUAL, and STRICT.
      • STRICT is the current default. There’s a visual modality curtain that darkens the view behind the dialog. Focus is trapped inside the dialog and pointer events are not passed through the curtain to background components. Requests to components behind that curtain are blocked unless there’s a low-level opt-in through e.g. DomListenerRegistration::allowInert or similar mechanisms for other request triggers.
      • VISUAL behaves like STRICT except that request are not blocked.
      • MODELESS doesn’t use modality at all – no modality curtain, no focus trap, no pointer event blocking, and no request blocking.
  • Change Dialog and ConfirmDialog to use VISUAL by default whereas LoginOverlay is STRICT by default.
  • Deprecate the void setModal(boolean) and boolean isModal() methods in Dialog. (The other two components don’t have this API but are currently always modal.)
  • Document this as a breaking change with potential security implications in release notes. Recommend users to update their code to explicitly use STRICT if they rely on the current security guarantees.

We plan to make this breaking change for Vaadin 25 unless this discussion gives a clear reason to reconsider.

2 Likes
2

Long story short: +1

Only suggestion: Make ConfirmDialog also STRICT by default.

3

Why? I think it’s more likely that a ConfirmDialog is used in a context that might be more sensitive to tampering through components behind the curtain. On the other hand, my gut feeling is that it’s just as problematic with desired background activity and the only reason we don’t notice is that it’s less frequently used.

4

My argument would be similar to LoginOverlay: I want the user to take an action without the possibility to bypass this Dialog.

1 Like
5

One thing that is missing is a suitable level of granularity. We now have control on a per-request-type basis which is inaccessible for application logic and almost always too granular. The suggested setModalityMode methods would provide control for each dialog instance which is probably too coarse.

The useful middle ground could be convenient control over all the request for a specific component instance: I don’t care about what happens with that Grid but I want to make sure the user cannot click this Button while the dialog is open above it. But even that would still leave the conflict between strict defaults and awareness.

6

To make sure we’re on the same page, can you explain MODELESS, STRICT and VISUAL modality modes?

7
  • STRICT is the current default. There’s a visual modality curtain that darkens the view behind the dialog. Focus is trapped inside the dialog and pointer events are not passed through the curtain to background components. Requests to components behind that curtain are blocked unless there’s a low-level opt-in through e.g. DomListenerRegistration::allowInert or similar mechanisms for other request triggers.
  • VISUAL behaves like STRICT except that request are not blocked.
  • MODELESS doesn’t use modality at all – no modality curtain, no focus trap, no pointer event blocking, and no request blocking.

Edit: also included this description in the original definition.

1 Like
8

Relevant issue: Provide a warning handler for Rpc Invocation Handlers warnings · Issue #20020 · vaadin/flow · GitHub

1 Like