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

Bug: Value-based permission checks do not roll back changes to the StreamModule #10

Open
robere2 opened this issue Feb 21, 2023 · 1 comment
Labels
Complexity: SECURITY This issue involves resolving issues which have security implications on the module or project Module: API This issue pertains to the apps/api module Priority: MED This issue isn't critical or security-related but is holding back other issues or efficiency. Type: Bug Something isn't working

Comments

@robere2
Copy link
Member

robere2 commented Feb 21, 2023

Normally, when a user mutates an object in the API, the flow looks something like this:

  • User submits request (Let's say it's a createUser request for this example)
  • API makes sure user has permission to create users
  • API makes sure user has permission to create all the fields supplied on Users
  • API creates the User via a Prisma transaction
  • API makes sure the user has permission to create the specific User object, with values, returned by Prisma.
  • If user has permission, the transaction is committed. If the user doesn't have permission, the transaction is rolled back.

With streams, they are not published through Prisma. This means that even if a user's value-based permission checks fail, the stream will still be created, as there is nothing to roll back. In other words, by the time the API checks if the user has permission to create the specific stream they are trying to create, it has already been sent off to the glimpse-video-control service for creation.

As an example, consider a user who has permission to stream with a to value pointing to youtube.com but not facebook.com. When the API checks the user's permissions, it only makes sure that the user has permission to use the to value before sending the value to the glimpse-video-control service. Later, the API will determine that the user doesn't have permission to send to facebook.com, but at that point it is too late, since the request has already been sent and there is no "rollback" feature.

This is not a major issue, since it only applies to modules which don't use the Prisma transaction (at the moment, only the StreamModule, which in our use-case does not have conditional permissions like this at the moment). A more general-purpose rollback mechanism should perhaps be created during the inevitable StreamModule rewrite.

@robere2 robere2 added Type: Bug Something isn't working Priority: MED This issue isn't critical or security-related but is holding back other issues or efficiency. labels Feb 21, 2023
@robere2
Copy link
Member Author

robere2 commented Feb 21, 2023

Note: The reason we do not check permissions before creating them is because services (i.e. Prisma) may generate or change the values supplied. We want to include these generated values in our permission checks.

@robere2 robere2 transferred this issue from rpitv/glimpse-api Jan 1, 2024
@robere2 robere2 added the Module: API This issue pertains to the apps/api module label Jan 1, 2024
robere2 added a commit that referenced this issue Jan 7, 2024
Somehow fixed footer sizing by just renaming the import
robere2 added a commit that referenced this issue Jan 7, 2024
Somehow fixed footer sizing by just renaming the import
@robere2 robere2 added the Complexity: SECURITY This issue involves resolving issues which have security implications on the module or project label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: SECURITY This issue involves resolving issues which have security implications on the module or project Module: API This issue pertains to the apps/api module Priority: MED This issue isn't critical or security-related but is holding back other issues or efficiency. Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant