imp:web: access control UX cleanups (fix #834) #2104
Conversation
Changes: 1. rename the sandstorm "manage" permission to "edit" (old permission names: view, add, manage; new permission names: view, add, edit). Rationale: "edit" best describes this permission's current powers, to users and to operators. If we ever added more manager-type features we'd want that to be a new permission, not a rename of the existing one (which would change the powers of existing users). 2. rename the sandstorm roles for consistency with permissions (old role names: viewer, editor, manager; new role names: viewer, adder, editor) Rationale: it's needed to avoid confusion. 3. add a new option: --allow=view|add|edit|sandstorm (default: add). 'sandstorm' sets permissions according to the X-Sandstorm-Permissions header. Drop the --capabilities and --capabilities-header options. Rationale: it's simpler and more intuitive. 4. replace "capability" with "permission" in ui/docs/code. Rationale: consistent with the above, more familiar.
simonmichael
added
web
|
Here's some cleanup for hledger-web access control and #834. I'll merge unless @zarybnicky or @ocdtrekkie see problems. |
|
Is there a particular reason to re-order the roles? I don't think it will matter, but we should definitely test it. I will try to get an experimental build within the next day for you based on this branch. |
|
Well spotted, yes I reordered the roles to be consistent with the "increasing power" order as in permissions and --allow option; I found it easier to work on. As you said, they warn about this for permissions and not for roles, and anyway nobody is using this yet, so I think it'll be fine. |
|
It'll be nice to test the theory we can reorder them so I'm cool with it. |
|
If you want to make this easily testable from your branch, you should edit .sandstorm/launcher.sh to use the new --allow option. And maybe increment the appVersion in the package definition file, though we only need to do that before we actually release the update. |
|
Good call - pushed. |
|
It looks like re-ordering the roles is a destructive change. The view-only link on the old PR now grants the editor access. |
|
Thanks for testing. Stupid thing. I vote to leave it tidy, as is, since we have nobody using the old order. |
|
That scares me in a "technically we have no way to know for sure" sense, but if that's how you want to proceed, can we increment the appVersion ASAP so I can push a new release as fast as possible? |
|
Pushed - thanks. |
The --capabilities option has been removed by upstream. simonmichael/hledger#2104
The --capabilities option has been removed by upstream. simonmichael/hledger#2104
Changes:
new permission names: view, add, edit).
Rationale: "edit" best describes this permission's current powers, to users and to operators. If we ever added more manager-type features we'd want that to be a new permission, not a rename of the existing one (which would change the powers of existing users).
new role names: viewer, adder, editor)
Rationale: it's needed to avoid confusion.
Rationale: it's simpler and more intuitive.
Rationale: consistent with the above, more familiar.
[5. reverse the order of role definitions.
Rationale: "increasing power" order is consistent with the permissions and --allow definitions, hence less confusing. We thought it wouldn't change the roles' meaning, but it did, but no-one is using this yet anyway.]