-
-
Notifications
You must be signed in to change notification settings - Fork 337
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
make access control for ConvertFromColumn action less brutal #1111
Conversation
d65275f
to
2c7af80
Compare
Access control for ConvertFromColumn in the presence of access rules had been left as a TODO. This change allows the action when the user has broad rights to the table it applies to.
8c1acac
to
6e00834
Compare
// If changes could include Python formulas, then user must have | ||
// +S before we even consider passing these to the data engine. | ||
// Since we don't track rule or schema changes at this stage, we | ||
// approximate with the user's access rights at beginning of | ||
// bundle. | ||
// We also check for +S in scenarios that are hard to break down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you point out what scenarios you have in mind here? I think mentioning the Copy/Transform action would be enough.
{id: 'C', type: 'Int'}]], | ||
['AddRecord', '_grist_ACLResources', -1, {tableId: 'Table1', colIds: 'C'}], | ||
['AddRecord', '_grist_ACLRules', null, { | ||
resource: -1, aclFormula: 'user.Access == OWNER', permissionsText: '-R', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you add this rule, I don't think it is exersiced anywhere below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need at least one rule. In the absence of any rules, SPECIAL_ACTIONS
are already allowed for editors, so the test would succeed trivially.
Added a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Access control for ConvertFromColumn in the presence of access rules had previously been left as a TODO. This change allows the action when the user has schema rights. Because schema rights let you create formulas, they let you read anything, so there is currently no value in nuance here.