web: Rename/alias --capabilities and --capabilities-header

[zarybnicky]

Follow-up to #821. [and issuecomment-401393071]

I don't like capabilities either, but it seemed like the best conceptual fit to what it does. There are two problems here - one is the naming, the other is UX.

--capabilities enables features of the web UI that are available always (unless overridden by --capabilities-header), --capablities-header specifies a header (X-Sandstorm-Permissions on Sandstorm), which contains the permissions that the current role can access. We could reuse the Sandstorm terminology and say that the web application started by hledger web --permissions=view,add,manage has the permission to edit the journal in its entirety. --access-level also seems to fit.

The UX problem is a different one. It is currently possible to start the webapp like hledger web --capabilities=manage, and the app would be broken (perhaps functional, but with a broken UI). Do we foresee a need for more permissions? (E.g. manage-users, switch-journal)

If yes, then there should be built-in roles for the static-permissions case (e.g. --role admin == --permissions=view,add,manage), so that starting the app with misconfigured permissions is not as easy. If these permissions are all that will ever be needed (if hledger-web's intent is to only be a simple web UI without too many features), then the solution could be same as above, or the permissions/access-levels could be gradual - manage will always imply both add and view, and add will imply view (a sort of implicit roles)

I'm unsure what the best solution here is. --role, --permissions, and --permissions-header uses commonly known terminology and is both accessible and extensible. Only --permissions and --permissions-header (or alternatives) without --role but with implicit roles is not as clearly extensible, but would be simpler.

(In writing this issue, I almost decided on --permissions (-P?), which seems to be a better alternative to --capabilities (which is a good conceptual fit but an uncommon term), --access-level (which works but is conceptually closer to role than to permission) or others.)

Any thoughts on this? I didn't expect my PR to get merged and immediately released, but I think that in any case, we need to keep --capabilities{,-header} at least until next release.

[simonmichael]

@zarybnicky , here's a very delayed answer, re renaming hledger-web --capabilities, after tripping up on it today. I propose we replace the command line option with --allow=view|add|edit. add includes view, edit includes view and add. manage could be added later if we ever support more than editing. I'll do this soon if you have no objection.

I don't have an opinion on whether it's worth renaming/changing --capabilities-header for sandstorm, maybe @ocdtrekkie does.

[ocdtrekkie]

I don't have a strong opinion on naming as long as it's functionally the same. (Sandstorm users never see this!)

I definitely agree it would be silly to let a user have manage rights but not view rights, so while it's nice to be able to specify overlapping rights and compose later. (Sandstorm's vision here is that different types of roles may have differing sets of permissions that need to be composed if someone has access to multiple, not just a view/edit/admin distinction.) In a more simple permissions case, I think it makes sense that if you have any of these capabilities, you can view, if you have manage, you can also edit, etc.

[simonmichael]

I pushed a start at this in the allow branch: 528b2b9 . I'll finish it later, unless you want to do the bit in Foundation.hs @ocdtrekkie .

[ocdtrekkie]

I don't know how to write Haskell anything. 🤭

[simonmichael]

--capabilities-header=HTTPHEADER
: read capabilities to enable from a HTTP header, like X-Sandstorm-Permissions (default: disabled)

Is there any point to being able to pick a different name for this HTTP header ? I assume Sandstorm calls it X-Sandstorm-Permissions and nothing else. Is it a common mechanism used by other web apps we want to support ? If not, should we merge this flag with the other one, eg it could be

--allow=view|add|edit|sandstorm

[simonmichael]

Also is the word capabilities important in this context ? I'm inclined to use "permissions".

[simonmichael]

https://docs.sandstorm.io/en/latest/developing/auth

X-Sandstorm-Permissions: This contains a list of the permissions held by the current user, joined with a comma such as edit,admin or just edit. Permissions are defined in the package's sandstorm-pkgdef.capnp. The grain's owner holds every permission and can use the "Share access" button to authorize other users.

[simonmichael]

@zarybnicky I see you shared thoughts on naming above. I think:

• permissions is a good familiar word
• users shouldn't have to manage individual permissions, instead we'll offer a choice of access level.
• let's call the flag --allow rather than --access-level, for easy typing and readability (even though it adds another word)
• it should accept a special value meaning "whatever sandstorm says", and we can drop the --capabilities-header option
• isn't our current X-Sandstorm-Permissions checking insecure, because anyone can add that plain text request header to give themselves any permission ? Am I wrong ?

  -- do some permissions checking
  access <- case capabilitiesHeader_ opts of
    Nothing -> return (allow_ opts)
    Just h -> do
      hs <- fmap (BC.split ',' . snd) . filter ((== h) . fst) . requestHeaders <$> waiRequest
      fmap join . for (join hs) $ \x -> case capabilityFromBS x of
        Left e -> [] <$ addMessage "" ("Unknown permission: " <> toHtml (BC.unpack e))
        Right c -> pure [c]

[ocdtrekkie]

@simonmichael It's not a security issue because you would only have the header option specified in the case you are using Sandstorm (or another header-based authentication proxy) which is writing to that header. All access to a Sandstorm app goes through Sandstorm, which writes the header.

[simonmichael]

I see, Sandstorm's http bridge sanitises incoming requests and always sets this header itself, replacing anything a user might try to put there.