New list permission to protect Summary pages

[ebruchez]

A user has the following requirement:

• some users are anonymous
• users can create (/new) and edit/view (only with "link provided")
• however, all Summary pages cannot be seen by anonymous users, so that users cannot discover existing data entered by other users, anonymous or not

At the web.xml level, we can protect, for example:

• /fr/: Home Page
• /fr/*: all Form Runner paths
• /fr/myapp/myform/summary: a specific Summary page

However, it's not possible to block all paths ending with /summary. This is just not a feature of the Servlet specification's <url-pattern>.

This issue is about finding a solution that enables the initial scenario above.

[ebruchez]

One issue is that our paths use the mode or page as a suffix: /new, /edit, /summary, etc. If we could make those prefixes, then we could match on them, for example:

• /fr/acme/sales/summary → /fr/summary/acme/sales
• /fr/acme/sales/edit/1234 → /fr/edit/acme/sales/1234

Would that generally be better, or would that cause more problems? In fact, there is a use case to match on the app name (here acme). That could be solved by enumerating the pages/modes, of which there are a limited number:

• /fr/new/acme/*
• /fr/edit/acme/*
• /fr/view/acme/*
• /fr/pdf/acme/*
• /fr/summary/acme/*
• and a few more

So possibly, switching paths around (optionally) might be an option, which would then allow more useful matches in web.xml.

[ebruchez]

If the web.xml solution doesn't work, then we need to do it from within Orbeon Forms.

We already have a mechanism for permissions, of course, at the form level, with a suggestion to extend that to per-app and global defaults (#1860).

However, what permission would apply in the case of the Summary page? Right now, the Summary page will filter individual form data based on individual permissions. In the specific case of the Form Builder Summary page, we can also send a 403 for the whole page in some cases, but I don't think that the case for Form Runner in general?

Now we should be able to either:

• not show the Summary page at all (preferably)
• or show it but with an empty list

If we had a separate list permission in addition to CRUD, this could be achieved fairly easily, by requiring that the user have a certain role (and therefore be logged in) to access the Summary page at all.

If so, #1860 would help.

[ebruchez]

What would it take to add a new list permissions?

• add List to Operation enum
• update Form Builder UI
• Summary page to make use of it
• Q: How to handle backward compatibility?
• doc
• Q: Should the Search API itself be protected?

It doesn't look very hard, but is it general enough to be worth it?

[avernet]

This will be handles once we finish the implementation of #2256, see list permission in UI.

[ebruchez]

I had forgotten that we had already considered the "List/search data" and even "Access PDF" permissions.

We might want to do the list permission before we complete #2256, as customer needs a solution probably sooner.

[ebruchez]

For backward compatibility:

• if no permissions are defined for a form, then no problem: list is just one of the permissions that are automatically ganted
• if permissions are defined for a form, then we need to differentiate between
  • 2021.1.x and earlier: list was implicit
  • 2022.x and newer: list is explicit

Distinct possibilities:

1. Use the Orbeon Forms version that last modified the form to discriminate
   • however this makes form definitions more ambiguous than they should be, maybe
   • also Form Builder/Admin page have to migrate permission to explicitly add the list role
2. Instead of list, use a -list, or nolist token

#2256 will rectify that and use a list permission again.

[ebruchez]

XML permissions look like this right now:

<permissions>
    <permission operations="read update delete">
        <group-member/>
    </permission>
    <permission operations="read update delete">
        <owner/>
    </permission>
    <permission operations="create"/>
</permissions>

The backward compatibility list permission should be added for those entries that have, specifically, at least read, but not others. For example the create line should not imply list.

Note that right now:

• update ⇒ read
• but not delete ⇒ update, which is probably ok

Listing implies showing some data and metadata, so we should have list ⇒ read.

[ebruchez]

Implementing read ⇒ list by default and supporting -list in Form Runner is quite simple.

[ebruchez]

Form Builder is a little more difficult!

Question: does it make sense for list to be available for "Owner" and "Group members"? read clearly makes sense. Right now, the Summary page uses the list permission only for roles-based permissions, so I guess not.

But from a UI perspective (haha), this then means there shouldn't be list checkboxes for these two rows, similarly to the missing checkboxes for create.

[ebruchez]

Implementation notes:

• The Form Builder UI code was modified:
  • "UI XML" is normalized when reading (no -list, for example)
  • conversion from "UI XML" to "form definition XML" now done in Scala
  • implied permissions are not present in the "UI XML" to clarify/simplify (this was inconsistent before: update ⇒ read was present, but permissions inherited from "Anyone" were not present)
• Wrote a unit test to check that we convert from "UI XML" to "form definition XML", and then ADTs, and then back, correctly

[ebruchez]

+1 from customer and +1 from customer

[ebruchez]

Documented

[ebruchez]

Backward compatibility issue: with 2021.1, Summary page also allows access IF:

• owner can read
• AND user can create

But with current 2022.1 changes, user won't have list so will not have access to Summary page.

RESOLUTION: list must not imply read.

RESOLUTION: any of create, read, update, or delete ⇒ list (unless -list).

[ebruchez]

RESOLUTION: It's ok to have Search API check for the list permission too.

[ebruchez]

• FB UI: list must not imply read
• FR: list must not imply read
• FR: any of create, read, update, or delete ⇒ list (unless -list)
• Summary page: must not link rows to /new, /edit or /view if user can list but not do any other ops
• test new logic

[ebruchez]

Noticing we haven't closed this. There are a couple of tasks above:

• about the Search API: protect or not?
• about missing tests