Skip to content
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

Design Meeting Permissions UI #4540

Closed
2 tasks
mattkrick opened this issue Jan 16, 2021 · 8 comments
Closed
2 tasks

Design Meeting Permissions UI #4540

mattkrick opened this issue Jan 16, 2021 · 8 comments

Comments

@mattkrick
Copy link
Member

When i want to restrict visibility of my meetings, I need a UI to adjust the functionality added in #4539

AC

  • The New Meeting page includes a visibility dropdown with the team.defaultScope as the default value.
  • if invitees is selected, then a multi-select of team members names & avatars (and emails?) is shown with a search bar to make it easy to find people

Estimate: 16 hours

@jordanh
Copy link
Contributor

jordanh commented May 4, 2021

See also: #3721, #4527, and #4688

@jordanh
Copy link
Contributor

jordanh commented May 4, 2021

After scrub, making discussion. @ackernaut will take a crack at opening a new issue to supersede this one

@mattkrick
Copy link
Member Author

@mattkrick
Copy link
Member Author

today, we have 5 explicit roles, which we could structure using an RBAC:

  • org billing leader
  • org member
  • team lead
  • team member
  • meeting member

we also have a bunch of relations that augment this (ReBAC). For example:

  • meeting facilitator
  • meeting creator
  • comment author
  • task assignee
  • template creator

we even have a few attribute based things (ABAC):

  • template scope (TEAM, ORG, PUBLIC)

to make this more complex, we'll use a recursive hierarchy when we build Team of Teams.

Best practice is to check for RBAC, then check ReBAC, then check ABAC. if the answer is still denied, recurse using the parent resource, if available (in this case, the parent would be a team that holds the current team resource).

we could build a table that looks like {target, source, relation} to check for permissions. this is what google does with Zanzibar. However, this means building the app around the perms. The course did a good job of explaining that this is often a premature optimization.

Instead, I say we opt for decentralized like how it is now-- using GraphQL types or even fields. We do this today at the top of the resolve function. We could move it to a higher-order function, or as a directive to make it more expressive though.

The problem we face is with mutations-- for performance, we'd like to use a dataloader to fetch the authorization resource (eg an OrganizationUser). If we share a dataloader between the resolve function & the authorization, then it's possible the enforcement will cache that resource & the resolve function wouldn't know it before it mutated the ground truth-- leaving us with a stale cache. for example, let's say a changeFacilitator mutation first checks userId === Meeting.facilitatorId, thus caching the meeting. Then, it updates the meeting object in the DB but it does not update that object in the dataloader.

Options:

  • we leave it as is. if the permissions requite fetching a resource & the mutation mutates that resource, we update the DB as well as the cache. con: strongly couples auth to resolution
  • the enforcement module can't access the DB, you have to pass it the fetched object. con: this leads to overfetching because e.g. let's say a rule is to check & see if someone is a team lead or billing leader. we'll have to fetch TeamMember & Org & OrgUser. If they're a Team lead, the logic short-circuits & we wasted the hits to get Org & OrgUser.
  • we don't use a dataloader for permissions on mutations. con: inefficient, no reuse.
  • we clear the dataloader after permissions checks. con: no reuse.
  • we check to see if a record exists in the dataloader & if so, we update that information when we update the DB. con: tightly coupled
  • clearAll after enforcement by default, but accept a persist param so the dev can optimize if needed.
  • the enforcement returns something (the cached record? a clearAll command?) that can be called.
  • make sure to call clear() when mutation the DB. con: error prone,a touch less inefficient than the current solution as it will require an extra read.
  • the resolution hits the cache of anything that could have changed. con: the dev needs to know all the things that the enforcement could have cached

@mattkrick
Copy link
Member Author

After reading the zanzibar paper (https://storage.googleapis.com/pub-tools-public-publication-data/pdf/41f08f03da59f5518802898f68730e247e23c331.pdf) I'm pretty confident that centralized ACLs are for google scale & not our scale.

The only way for them to handle complex rules is to replicate the entire (trillion+) rules across 10+ datacenters. This makes sense for them because they have the resources operate more like a social network than a saas. we can keep rules on the objects themselves & then shard those across datacenters & thus be OK.

First, it helps establish consistent
semantics and user experience across applications.

Totally valid! Forced semantics are great

Second,
it makes it easier for applications to interoperate, for example, to coordinate access control when an object from one application embeds an object from another application.

Keep the attributes on the objects themselves & it makes it even easier. I call BS on this arg.

Third,
useful common infrastructure can be built on top of a unified
access control system, in particular, a search index that respects access control and works across applications.

If we had really convoluted rules I could see this making sense, but e.g. finding a keyphrase across all Tasks for all of the Users teams is pretty easy

@jordanh jordanh removed this from the Dashboard IA milestone Aug 5, 2021
@ackernaut
Copy link
Member

See discussion #5670

@github-actions
Copy link
Contributor

Stale issue

@github-actions github-actions bot added the stale label May 17, 2022
@jordanh jordanh added icebox and removed stale labels Jun 21, 2022
@jordanh
Copy link
Contributor

jordanh commented Jun 21, 2022

Iceboxing

@jordanh jordanh closed this as completed Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants